-
Notifications
You must be signed in to change notification settings - Fork 0
ETT-324: update account dropdown UI #120
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
Conversation
79811d8
to
7f0ab1b
Compare
} else if (loggedIn && role) { | ||
HT.login_status.institutionName = 'Moosylvania State'; | ||
HT.login_status.institutionCode = 'state'; | ||
HT.login_status.r = { [role]: hasActivatedRole }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PingCallbackDecorator to include the r
property of HT.login_status
to mock role status for storybook.
decorators: [ | ||
() => ({ | ||
Component: PingCallbackDecorator, | ||
props: { loggedIn: true, role: 'resourceSharing', hasActivatedRole: false }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and here is where the props for the role/status are passed in to PingCallbackDecorator
for this story (and the other stories that have a role, whether activated or not).
7f0ab1b
to
e839ca7
Compare
e839ca7
to
c5f06b1
Compare
Note to self: don't forget to build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the UI changes look great. I noted an odd thing on Firefox and Brave: in neither could I tab from "My Collections" to "Switch Role" -- focus jumps straight to "Sign Out". Don't know if that's a show-stopper so I'll just APPROVE noting potential head-scratchery.
(I should note that I was testing on dev-3)
@moseshll Thanks for catching that! I'll get that fixed up and then merge/deploy. |
@moseshll I had merged ETT-324 and ETT-347 on dev-3 so Gayathri and Ange could see the whole thing in action, and some weird rebase overwrite caused the dropdown tabbing issue. I staged ETT-324 on its own on dev-3 and had no issues tabbing to "Switch Role." When I rebase ETT-347, I'll double check this isn't an issue on that branch. Thanks again for catching that! |
It looks like a lot changed here, but I mostly re-arranged markup that already existed and changed a little bit of CSS/bootstrap utilities. I also added new stories! See the Chromatic build below for more on those.
This is staged on dev-3. All of these views can be reviewed in storybook (some of us have RS access turned on, none of us will log in as a regular member... unless you want to pull this down locally and use
switch_auth
).Views that need testing: