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 replace <AlertBanner> component #21375

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

hellobontempo
Copy link
Contributor

@hellobontempo hellobontempo commented Jun 21, 2023

new component

Screenshot 2023-06-12 at 11 49 51 AM

old component

Screenshot 2023-06-12 at 11 49 39 AM

hellobontempo and others added 5 commits June 14, 2023 14:39
* rename test selector

* replace db banner

* add class

* replace db role edit

* db creds

* generate creds

* simpler class

* license banner component

* oidc callback plash

* raft

* aws

* secret create or update

* change to compact alert for form field

* change back to inline

* combine alert banners

* wrap in conditional

* remove references to message class
* token expire warning

* delete css

* edit form

* item details distribute mfa step 2 transit verify

* back to secondary

* distribute

* oidc lease error

* sign

* kv obj and repl dash

* more repl

* update test selector

* show, creds

* shamir

* pki csr

* pki banners

* add hds library to ember engines

* woops comma

* fix k8 test

* update message error component for last!

* hold off MessageError changes until next pr

* revert test selectors

* update pki tests
* final component swap

* and actual final of MessageError

* update MessageError selectors

* delete alert-banner and remove references

* update next step alerts to highlight color

* finishing touches, auth form test and client dashboard inline link

* fix more selectors

* fix shamir flow test
* replace AlertPopup

* add test tag

* move tag

* one more message error tag

* delete alert popup

* final css cleanup

* move preformatted flash into <p> tag
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-06-13 at 4 32 51 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

combined banners into one message (pattern we used in client count dashboard)
Screenshot 2023-06-13 at 4 23 44 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved <section> and <div> here so the alert spans the full UI width

image

<button onclick={{action "enable" @model}} type="button" class="link" data-test-enable={{true}}>
Enable
</button>
<A.Button data-test-enable-identity @text="Enable" @color="secondary" {{on "click" (action "enable" @model)}} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consulted design and intentionally changed this button to look like a button (not a link)
Screenshot 2023-06-14 at 4 04 17 PM

secondary class is for alert action buttons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

class="message" defaults to the old structure's info blue background, so this looked a little incohesive beside the new alert banner:

before

image

after:

updated to generic grey and removed the erroneous div for now. design has a ticket to revisit this
Screenshot 2023-06-15 at 3 41 00 PM

</LinkTo>
to enable tracking again.
</AlertBanner>
<Hds::Alert @type="inline" @color="warning" class="has-bottom-margin-s" 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-06-16 at 4 33 12 PM

<li>
{{this.upgradeVersionAndDate}}
{{this.upgradeExplanation}}
<DocLink
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plan is to keep <DocLink> but use the HDS component in that template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Design decided @color="highlight" is more appropriate for Next steps banners
Screenshot 2023-06-16 at 4 57 31 PM

@@ -28,11 +28,11 @@
@color={{this.tidyStateAlertBanner.color}}
@icon={{this.tidyStateAlertBanner.icon}}
class="has-top-margin-m"
data-test-hds-alert
data-test-tidy-status-alert
Copy link
Contributor Author

Choose a reason for hiding this comment

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

trying to make selectors more component-relevant so debugging is easier in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flash message comparisons new + old

purple highlight for info

Screenshot 2023-06-20 at 12 09 42 PM


wrapping of really long item names

Screenshot 2023-06-20 at 12 10 02 PM


applying white-space: pre-wrap class and moving inside <p> tag

Screenshot 2023-06-20 at 12 10 29 PM

The input is
{{if @valid "valid" "not valid"}}
for the given
{{if @signature "signature" "HMAC"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe, because this is a description, both signature and HMAC need a period?

Copy link
Contributor

Choose a reason for hiding this comment

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

I know you didn't change/write the copy, but if we're in there, might as well address some punctuation.

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.

Nice work! Nothing blocking, just a couple of clean up comments. And gosh do these look sooo much better!

* add periods, move link to trailing

* more periods and typo fix
@hellobontempo hellobontempo merged commit 76e742b into main Jun 21, 2023
@hellobontempo hellobontempo deleted the ui/VAULT-16910/hds-adoption-replace-AlertBanner branch June 21, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants