UI: Part 1 - hds adoption replace <Modal> #23363
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
There was a problem hiding this comment.
Cleaned up this view a bit while I was here 🧹
| @@ -70,36 +82,6 @@ | |||
| <span class="is-size-9 has-text-grey has-bottom-margin-l"> | |||
| You can use Alt+Tab (Option+Tab on MacOS) in the code editor to skip to the next field. | |||
| </span> | |||
| {{! Only renders button (and modal) if not already in the "create policy" modal }} | |||
There was a problem hiding this comment.
These links were incorrect - this copy and correct links already render below the example policy in the modal
| @@ -23,11 +23,24 @@ | |||
| </div> | |||
| {{/if}} | |||
| <div class="field"> | |||
| {{#if @model.isNew}} | |||
| <label class="has-text-weight-bold">Policy</label> | ||
| <ToolbarActions> | ||
| <div class="toolbar-separator"></div> | ||
| <Toolbar> |
| {{else}} | ||
| {{! EDITING - no file upload toggle}} |
There was a problem hiding this comment.
no need to render another JsonEditor, the only difference between create/edit is create has a file upload toggle and edit renders a copy button
| @@ -15,14 +15,12 @@ | |||
| </div> | |||
| <div class="header-right"> | |||
| {{#if this.hasCsvData}} | |||
| <button | |||
| <Hds::Button | |||
| {{#if this.showCSVDownloadModal}} |
| @@ -91,41 +89,40 @@ | |||
| </div> | |||
|
|
|||
| {{! MODAL FOR CSV DOWNLOAD }} | |||
| <Modal | |||
| @@ -128,26 +110,26 @@ | |||
| </div> | |||
| </div> | |||
| </form> | |||
| {{! SAMPLE POLICY MODAL. Only renders modal if not already in create policy modal }} | |||
| {{#if @renderPolicyExampleModal}} | |||
There was a problem hiding this comment.
this arg is only used to hide/show the Modal toggle button in the toolbar above
| @@ -47,47 +47,34 @@ | |||
| </div> | |||
| </form> | |||
|
|
|||
| <Modal | |||
| </button> | ||
| </footer> | ||
| </Modal> | ||
| {{#if this.modalOpen}} |
| @@ -29,6 +29,15 @@ | |||
| Scope | |||
| </h1> | |||
| </p.levelLeft> | |||
| <p.levelRight> | |||
There was a problem hiding this comment.
Design recommended moving the button to the header (before it was an inline link below the JsonEditor)
There's a polish task to do the same for the policy form, but the header is in the route file which makes this change outside the scope of this PR - so it's in the Json editor toolbar for now.
| > | ||
| <section class="modal-card-body"> | ||
| <div class="is-flex-between is-flex-center has-bottom-margin-s"> | ||
| {{#if this.showTemplateModal}} |
| <p data-test-modal-text> | ||
| Example of a JSON template for scopes: | ||
| </p> | ||
| <Hds::Copy::Button |
There was a problem hiding this comment.
Removed this copy button and removed @showToolbar={{false}} from the JsonEditor to align with the other places we use the JsonEditor to render sample templates
| @@ -78,40 +80,25 @@ | |||
| {{/if}} | |||
| </form> | |||
|
|
|||
| <Modal | |||
There was a problem hiding this comment.
styling changes are intentional, the floating copy button (with no toolbar) was anti-pattern
|
Build Results: |
| @@ -53,14 +53,12 @@ | |||
| {{/if}} | |||
| {{#if this.capabilities.canUpdate}} | |||
| {{#if (gt this.model.allowed_roles.length 0)}} | |||
| <button | |||
| > | ||
| Edit transformation | ||
| </button> | ||
| <Hds::Button |
| @@ -96,30 +95,27 @@ | |||
| <MessageError @model={{this.model}} @errorMessage={{this.error}} /> | |||
| </ConfirmationModal> | |||
|
|
|||
| <Modal | |||
| {{#if this.isEditModalActive}} |
| @@ -386,3 +386,17 @@ a.button.disabled { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| .hds-button { | |||
There was a problem hiding this comment.
Doing the same thing as we did for copy button, making new classes to match structure. When we remove finally .button class we can audit these with design
There was a problem hiding this comment.
Ok so this is to preserve the existing styling but ultimately we will want consistent HDS styling once the button component is adopted and this can be removed? If so a comment might be helpful just so it's easy to spot when it comes time to remove.
There was a problem hiding this comment.
yep - there's a comment for this section above the .hds-copy-button class changes. I can add another here
| @@ -386,3 +386,17 @@ a.button.disabled { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| .hds-button { | |||
There was a problem hiding this comment.
Ok so this is to preserve the existing styling but ultimately we will want consistent HDS styling once the button component is adopted and this can be removed? If so a comment might be helpful just so it's easy to spot when it comes time to remove.
Yeah - wrapping the new modals in conditionals really bonks up the diff 🥲 |
| @@ -64,7 +64,6 @@ module('Integration | Component | client count config', function (hooks) { | |||
| this.createModel('disable'); | |||
|
|
|||
| await render(hbs` | |||
| <div id="modal-wormhole"></div> | |||
There was a problem hiding this comment.
I think this was talked about in one of the threads in our channel, but I can't seem to remember. With the adoption of hds modals, will all the modal-wormhole be removed in tests?
There was a problem hiding this comment.
Yep! Modals are renders in place, inside a conditional, so there's no "other" location for them! I was going to remove these divs all at the end, but figured if I'm updating tests I might as well remove them as I go
hellobontempo
merged commit fc7ded2
into
ui/VAULT-17315-hds-adoption-replace-Modal
* UI: Part 1 - hds adoption replace <Modal> (#23363) * replace policy-form modal * replace clients/attribution modal * clients/config modal * scope form odal * remove button type * include toolbar to match other example templates * rotate credentials modal * add toolbar button class for hds buttons * transformation-edit modal * add back test selector * add route arg to button! * update link status * fix link-status tests * remove prevent default * update db tests * update tests * use page alert for hcp link status banner * fix scopy button selector * fix sidebar test * change to neutral banner * UI: Part 2 - hds adoption replace <Modal> (#23398) * upgrade HDS library (adds support for snippet containers * cleanup flight icons * replace transit key action modals * re-add deps as devDeps * remove line * address transit tests * UI: Part 3 - hds adoption replace <Modal> (#23415) * cleanup css * cleanup extra type attr * masked input download modal * use Hds::Button in download button" * fix size of modal * tiny icon fix * refactor download button to always render download icon * update tests * UI: Part 3.5 - hds adoption replace <Modal> (#23448) * replication-promote modal * replication component modals * replication add secondary modal * move update text for diff * UI: Part 4 - hds adoption replace <Modal> (#23451) * k8 configure modal * kv delete modal * ldap modals * pki modals * add trash icon * move deps * UI: Part 5 - hds adoption replace <Modal> (#23471) * replace confirmation modals --------- * UI: Part 6 - hds adoption replace <Modal> (#23484) * search select with modal * policy search select modal * replace date dropdown for client dashboard * change padding to top * update policy example args * lolllll test typo wow * update dropdown tests * shamir flow modals! * add one more container * update test selectors * UI: Final hds adoption replace <Modal> cleanup PR (#23522) * search select with modal * policy search select modal * replace date dropdown for client dashboard * change padding to top * update policy example args * lolllll test typo wow * update dropdown tests * shamir flow modals! * add one more container * update test selectors * remove wormhole and modal component * fix selectors * uninstall wormhole * remove shamir-modal-flow class * fix confirm modal test * fix pki and kv test * fix toolbar selector kv * client and download button test * fix-confirmation-modal-padding * fix replication modal tests so relevant modal opens (#23540) * more confirmation modal tests * adds changelog
🔄 Replaces modal in
policy-form.hbsclients/attribution.hbsclients/config.hbsdatabase-connection.hbslink-status.hbs* pending design questions with this onescope-form.hbstransformation-edit.hbs