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: [VAULT-17035 VAULT-17038 VAULT-17039] Dashboard Quick Actions card #21929

Merged

Conversation

kiannaquach
Copy link
Contributor

@kiannaquach kiannaquach commented Jul 18, 2023

Quick Actions Card
radio

TODOs:

  • Quick Actions acceptance tests! I created some acceptance tests, but they caused other tests in the app to fail. In order to move along with this project, I created a ticket (VAULT-18885) to figure out why the acceptance tests are causing other app test failures later.

@kiannaquach kiannaquach added this to the 1.15 milestone Jul 18, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 18, 2023
@github-actions
Copy link

CI Results:

All Go tests passed! ✅

@github-actions
Copy link

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Left some comments! Let me know if you have any questions! Nice work so far - this little quick actions card has a lot going on!

ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
@kiannaquach kiannaquach marked this pull request as ready for review August 8, 2023 07:23
@kiannaquach kiannaquach changed the title UI: Dashboard Quick Actions card UI: [VAULT-17035 VAULT-17038 VAULT-17039] Dashboard Quick Actions card Aug 8, 2023
case 'Issue certificate':
return {
title: 'Role to use',
placeholder: 'Type to find a role...',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it was in the design, but I don't feel like you should have ellipsis at the end of "Type to find a role". Ellipsis indicate an omission of words, which I don't feel like we have here. The placeholder for Certificate serial number makes sense to have an ellipsis but I would say it's complete as "Type to find a role"

Copy link
Contributor

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

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

Looks beautiful! 🤩 great work

ui/app/components/dashboard/quick-actions-card.js Outdated Show resolved Hide resolved
@disallowNewItems={{true}}
@fallbackComponent="input-search"
@onChange={{this.handleSearchEngineSelect}}
@placeholder="Type to select a mount..."
Copy link
Contributor

@Monkeychip Monkeychip Aug 8, 2023

Choose a reason for hiding this comment

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

same comment here about ellipsis. If it's on the designs, I feel like it's worth clarifying with them if it makes sense in the cases I'm pointing out.

Copy link
Contributor Author

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! I'll remove the ellipsis for now, and ask design about it!

Copy link
Contributor Author

@kiannaquach kiannaquach Aug 8, 2023

Choose a reason for hiding this comment

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

I just realized that the PKI overview card SearchSelect placeholders use ellipsis, I will still remove for now and will clarify w/ design @Monkeychip

Screenshot 2023-08-08 at 9 09 00 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - I was under the impression that ellipses were our pattern for search select place holders

@onChange={{this.handleActionSelect}}
@fallbackComponent="input-search"
@nameKey={{this.searchSelectParams.nameKey}}
@disabled={{not this.searchSelectParams.model}}
Copy link
Contributor

Choose a reason for hiding this comment

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

nice and clean on this. Looks great!

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, a couple of non-blocking comments.

@kiannaquach kiannaquach merged commit a9603f6 into ui/landing-page-dashboard Aug 8, 2023
@kiannaquach kiannaquach deleted the ui/dashboard-quick-actions-card branch August 8, 2023 16:31
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.

4 participants