feat: update mgt-login to new fluent-ui designs #1807
Conversation
|
Thank you for creating a Pull Request @gavinbarron. This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:
|
|
The updated storybook is available here |
There was a problem hiding this comment.
Hover on accounts on multi-login is missing a pointer
Spacing between the signout button and main profile avatar is abit off, increasing the margin-top of main-profile to 27px meets the design
Spacing between the main profile and other accounts is also abit off, increasing the margin-bottom of the main profile to 47px meets the design
Show presence login story displays the vertical version and the presence login is missing.
ghost
added
the
|
The updated storybook is available here |
Yes, the vertical style card is shown when the Thanks for the hover state and spacing catches, will update those :) |
|
@Mnickii I took a second look at the spacing of the elements in the designs as using those numbers raw looked a bit off to me. When using a horizontal person the spacing from the button to the person appears to be 12px When using a vertical person the spacing from the button to the person appears to be 21px For the spacing between the vertical avatar and the first account in the list there is 47px of gap in the designs: Unfortunately, with this element we have margin/padding from both the avatar elements and the list-box elements that need to be accounted for when calculating this gap, based on my crude ruler work I think we need 6px more on the bottom margin of the main-profile: I'll work on getting these changes applied here. |
|
@gavinbarron for multiple accounts, how do you enable that? The multiple account story allows me to "sign in with a different account"
However, the component in the normal render mode just renders the signed in account, when I want to add another account, how do I sign into another account?
|
|
@musale as discussed in person, due to the way the MockProvider that is used to provide data to storybook works the multi login story is informational to show the expected experience, we can't actually enable login or account switching, but I'll add a note into that story. The provider being used is what controls the multi account behavior, checkout the JS tab for how the MockProvider is being configured in the multi account story: And yes, I do need to add the docs PR thanks for the reminder. |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
* feat: update mgt-login to fluent-ui designs * tests: added story for multiple logged in accounts * fix: use correct import for MockProvider initialization * fix: correct spacing and hover colors * updating spacing, and mgt-person hover states * docs: correcting doc comments and fixing small style issue * adding retries to yarn install for azure devops * clean up button colors, spacing, and documentation * Ensuring doc comments are provided where needed
* feat: update mgt-login to fluent-ui designs * tests: added story for multiple logged in accounts * fix: use correct import for MockProvider initialization * fix: correct spacing and hover colors * updating spacing, and mgt-person hover states * docs: correcting doc comments and fixing small style issue * adding retries to yarn install for azure devops * clean up button colors, spacing, and documentation * Ensuring doc comments are provided where needed
Closes #1627
PR Type
Description of the changes
PR checklist
yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense)Other information