-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Remove custom tag class and replace with Hds::Badge #29475
Conversation
Build Results: |
CI Results: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on your first PR! 🎉 Excellent attention to detail. There are just a few cleanup items to address. I didn't comment on all of them, but we'll also want to remove any has-text-<color>
classes from the added badges.
It might be nice to update the title to mention that we're replacing it with the hds component and removing the tag
class all together. Something like UI: Remove custom tag class and replace with Hds::Badge
.
There are also some tests failing, so we need to address those. Let me know if you want to pair through running tests locally!
<Hds::Badge @text="admin" /> | ||
policy such that a user named "James Thomas" or has the | ||
<code class="tag is-marginless is-paddingless">Team Lead</code> | ||
<Hds::Badge @text="Team Lead" /> | ||
role can manage the | ||
<code class="tag is-marginless is-paddingless">admin</code> | ||
<Hds::Badge @text="admin" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should all be code elements like the one below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version | ||
{{or @model.version "1"}} | ||
</span> | ||
<Hds::Badge @text={{concat "version " (or @model.version "1")}} data-test-kv-version-badge /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another one that doesn't need concat
if you prefer to write it this way:
<Hds::Badge @text={{concat "version " (or @model.version "1")}} data-test-kv-version-badge /> | |
<Hds::Badge @text="version {{or @model.version '1'}}" data-test-kv-version-badge /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done! Great work on your first PR 🎉 excellent description and inclusion of screenshots! ✨ Before merging, we want to make sure enterprise tests pass so we don't accidentally break enterprise when this is merged there
I went ahead and added the |
Description
Replaces instances of the
tag
class in favor ofHds::Badge
and removes associated styles. For most of these, the badge is identical. A few may now have improved spacing.This also establishes a pattern for code elements within a larger surrounding text. In these cases, we will use
![image](https://private-user-images.githubusercontent.com/195936383/410583390-8e9424f7-549c-4669-aada-c1922cd711a7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkxODM1MTksIm5iZiI6MTczOTE4MzIxOSwicGF0aCI6Ii8xOTU5MzYzODMvNDEwNTgzMzkwLThlOTQyNGY3LTU0OWMtNDY2OS1hYWRhLWMxOTIyY2Q3MTFhNy5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjEwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMFQxMDI2NTlaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT00NTJhNmNiZTljZjNkNWZmNjNmZmQ0NDk2NWViYTViY2YxMmRjMDI4YmM0M2NhYTE2NWJlMGQwODFlOWJjMDBkJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.d1IdgnouB1lCc_SKbZDLNSNw3eearzNxhRV8YvU_U0o)
Hds::Text:Code
with classcode-in-text
to have a uniform element appearance.Example:
Before:
After:
TODO only if you're a HashiCorp employee
backport/
label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x
, but older release branches will bebackport/ent/x.x.x+ent
.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.