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 <CopyButton> part 3 #22614

Conversation

malinac02
Copy link
Contributor

@malinac02 malinac02 commented Aug 29, 2023

This is the 3rd of 5 subtasks for the replacement of the structure <CopyButton> with the <Hds::Copy::Button>. In this subtask, I replaced the copy buttons in 7 files, namely the files in ui/app/templates/components/transit-key-action. Photos of each before and after are shown in the comments.

Sticking with the plan of making the HDS copy button look similar to the structure copy button until all buttons are replaced, I created a class to mimic the old style of the "Copy & Close" button you'll see in 6 of the 7 changed files. After this PR and all other subtasks are merged into my side branch, I will get feedback from Design about the implementation.

NOTE:
The icon of the <Hds::Copy::Button> changes to show success or error when it is clicked. Therefore, with the HDS copy button, there are no longer flash messages for success and error.
But, for the Copy & Close buttons you'll see in this PR, I kept the flash message. This is because when you Copy & Close, the modal closes so you aren't able to see the success icon on the copy button

@malinac02 malinac02 added ui pr/no-changelog hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed labels Aug 29, 2023
@malinac02 malinac02 added this to the 1.16 milestone Aug 29, 2023
Copy link
Contributor Author

@malinac02 malinac02 Sep 1, 2023

Choose a reason for hiding this comment

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

Before:
Before (output format - plaintext)

After:
After (output format - plaintext)

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:
Before
After:
After

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 and after look the same as decrypt.hbs

Copy link
Contributor Author

@malinac02 malinac02 Sep 1, 2023

Choose a reason for hiding this comment

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

Before:
The styling here was messed up, unlike the other files. Here is what it looked like before
Before
and here is it after scrolling to the right
Before 2

After:
I replaced the copy button and fixed the styling to match the style of the copyable content in the other transit key actions
After 2
I noticed that the content moves to the next line when there is a dash -. There is still the same scrolling functionality but it's own two lines. Is this meant to be like this?
After 1

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 and after look the same as decrypt.hbs

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 and after look the same as decrypt.hbs

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 and after look the same as decrypt.hbs

@malinac02 malinac02 marked this pull request as ready for review September 1, 2023 22:35
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Build Results:
All builds succeeded! ✅

&.white-icon svg {
color: $white;
}

&.is-primary {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we add a new is-primary class here? Looking at our current buttons stylesheet it looks like we have an is-primary, so curious if we want to have two different primary colors.

Copy link
Contributor Author

@malinac02 malinac02 Sep 6, 2023

Choose a reason for hiding this comment

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

Yes, I added a new is-primary class, but this one is a child of the hds-copy-button class as opposed to being a child component of the button class. This way, when I used this class on the HDS Copy Button, I would do class="hds-copy-button is-primary" rather than class="button is-primary hds-copy-button white-icon is-flex-center".

This is-primary class I created takes a few necessary styles from the button class, such as height and min-width, and the rest of the styles are borrowed from the other is-primary class. The class I made doesn't have anything like disabled, hover, active, etc since I didn't want to add too much extra styling to the HDS copy button.

Anyways, that was my thinking in creating a new is-primary class as a child of hds-copy-button class. I wanted to use less class names and avoid using the button class and its children for any HDS copy buttons. Please let me know if I should change this! This was just my intuition for how to do the styling, but I am not sure if there are better options (such as using the button is-primary classes). I'm also not sure if I should add the styling for hover, disabled, etc to the new is-primary class, if I keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I assume all this styling will change later, too, to better match the HDS designs. For now I'm trying to use the HDS copy button but match how the structure copy button looked)

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense to me - Thanks for clarifying Malina! 😊 I was curious since having two is-primary classes that essentially have very similar styles seemed like we could reuse the nested is-primary from the button class. However, I just pulled down your branch and saw you have a specific section for hds-copy-button. As you said, the styling will likely be removed when the copy buttons get refactored 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.

Sounds good! 😄

@malinac02 malinac02 merged commit 0045a29 into ui/VAULT-18829/hds-adoption-replace-CopyButton Sep 6, 2023
@malinac02 malinac02 deleted the ui/VAULT-19014/replace-CopyButton-part-3 branch September 6, 2023 18:39
@malinac02 malinac02 modified the milestones: 1.16, 1.15 Sep 6, 2023
@malinac02 malinac02 added this to the 1.16 milestone Sep 8, 2023
hellobontempo added a commit that referenced this pull request Sep 15, 2023
* Part 1: Upgrade HDS to 2.9.0 (#22311)

* UI: HDS adoption replace <CopyButton> part 2 (#22356)

* certificate-card.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* scope-form.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* fix tests caused by changing certificate-card. change hds copy button in certificate-card.hbs

* json-editor.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* masked-input.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* fix error with certificate-card.hbs copy button

* fix tests that deal with certificate-card.hbs

* add class to hds copy buttons to maintain similar styling to curent UI

* info-table-row.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* undo change that should instead by merged in from main

* change tooltip copy button to white. cleanup

* add extra tet for oidc scope form. edit css class for the white icon copy button

* fix tests

* UI: HDS adoption replace <CopyButton> part 3 (#22614)

* encrypt.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* decrypt.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* datakey.hbs. replace 6 <CopyButton> with <Hds::Copy::Button>

* rewrap.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* hmac.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* fix typo

* add copy-close class to copy & close buttons

* export.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>. fix styling

* sign.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* fix test caused by changing <pre> tag to <code> in export.hbs

* rename class

* add extra style to class needed for part 4 of copy button replacement

* UI: HDS adoption replace <CopyButton> part 4 (#22749)

* user-menu.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* transit-form-show.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* configure-ssh-secret.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* tool-hash.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* tool-random.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* tool-rewrap.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* tool-unwrap.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* tool-wrap.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* paths.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* code-snippet.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* cleanup css for code-snippet. add comments for getting rid of code-snippet and replacing with <Hds::Copy::Snippet

* change code-snippet copy icon to gray to match original design

* change code-snippet class

* accounts.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* hover-copy-button.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* add.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* show.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* copy-secret-dropdown.hbs: replace 1 <CopyButton> with <Hds::Copy::Button>

* change styling of 'link' copy buttons

* generate-credentials.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* transform-show-transformation.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* sign.hbs: replace 2 <CopyButton> with <Hds::Copy::Button>

* hide some copy buttons' icons and use original flash message

* undo cleanup of scss file so that I can put cleanup all into one PR to be more organized

* update code snippet copy button

* UI: HDS adoption replace <CopyButton> part 5: Cleanup (#22884)

* remove unecessary code-snippet.scssn class

* remove copy classes from masked-input.scss

* remove copy button class from text-file.scss

* uninstall ember-cli-clipboard 0.16.0 since there is no longer structure <CopyButton>

* remove copyright message from code-snippet.scss to avoid merge conflicts with main, where the file is deleted

* replace 2 classes with one

* remove unecessary class from copy button

* cleanup classes

* revert changes to avoid merge conflicts

* remove is-block class

* conditionally render private key

* add more info to comment

* remove HoverCopyButton

* add missing selector

* fix control group padding

---------

Co-authored-by: clairebontempo@gmail.com <clairebontempo@gmail.com>
Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>

* rename class to transparent background

* remove unused test selectors

* replace transit actions with Copy::Snippet

* replace transfrom code blocks with code snippet component

* revert extra css fiddling

* misc cleanup, unused action

* remove copy & close buttons from transit modals

* remove is- from class naming

* remove hds-copy-button class

* add other grey class

* more small cleanup

* add -top to margin

* add changelog

---------

Co-authored-by: clairebontempo@gmail.com <clairebontempo@gmail.com>
Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>
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 pr/no-changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants