Skip to content
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: HDS adoption update <AlertInline> component to use Hds::Alert #24299

Merged
merged 12 commits into from
Dec 1, 2023

Conversation

hellobontempo
Copy link
Contributor

@hellobontempo hellobontempo commented Nov 29, 2023

Use Hds::Alert @type="compact" in the AlertInline component so styling is consistent throughout UI.

Screenshot 2023-11-30 at 11 22 50 AM

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Nov 29, 2023
@@ -22,8 +22,7 @@
/>
</div>
<AlertInline
@sizeSmall={{true}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class size-small no longer exists (functionality have been removed previously?), so remove this arg all together.

@@ -34,5 +34,5 @@
/>
</Hds::SegmentedGroup>
{{#if this.invalidDate}}
<AlertInline @type="danger" @message={{this.invalidDate}} @paddingTop={{true}} @mimicRefresh={{true}} />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decided to make @mimicRefresh part of the default functionality in this component, since moving forward Hds::Alert should just be used directly if the refresh functionality is not needed

Comment on lines 56 to 60
<Icon class="is-flex-v-centered has-text-highlight" @name="alert-triangle-fill" />
<Hds::Alert class="is-flex-v-centered" @type="compact" @color="warning" />
{{/if}}
</div>
{{/each}}
{{#if this.indicesWithComma}}
Copy link
Contributor Author

@hellobontempo hellobontempo Nov 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before

Screenshot 2023-11-30 at 11 41 46 AM

after

Updated to match KvObjectEditor which uses @type="inline"
Screenshot 2023-11-30 at 11 41 21 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated
Screenshot 2023-11-30 at 4 34 09 PM

@hellobontempo hellobontempo marked this pull request as ready for review November 30, 2023 19:43
@hellobontempo hellobontempo added this to the 1.16.0-rc1 milestone Nov 30, 2023
Copy link

github-actions bot commented Nov 30, 2023

Build Results:
All builds succeeded! ✅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before/after

Screenshot 2023-11-30 at 1 08 09 PM

Your data contains an upgrade.
<DocLink
@path="/vault/docs/concepts/client-count/faq#q-which-vault-version-reflects-the-most-accurate-client-counts"
<Hds::Alert class="has-top-padding-m" @type="compact" @color="warning" as |A|>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-11-30 at 12 59 39 PM

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of non-blocking comments. Awesome work on this. It's going to look so good with this now in place!

@hellobontempo hellobontempo enabled auto-merge (squash) December 1, 2023 00:36
@hellobontempo hellobontempo merged commit 61ee28b into main Dec 1, 2023
71 checks passed
@hellobontempo hellobontempo deleted the ui/VAULT-17371/hds-adoption-replace-AlertInline branch December 1, 2023 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants