Enhance Login and Feedback Navbar Buttons #903
Conversation
| <NavItem {...commonProps} onClick={onSignInClick} title={loginText}> | ||
| <UserIcon /> | ||
| <InvisibleA11yLabel>{loginText}</InvisibleA11yLabel> |
There was a problem hiding this comment.
Instead of importing the a11y label could you just add aria-label to the NavItem? Same with the survey button.
There was a problem hiding this comment.
That's the advantage of using the invisible element: no extra ARIA attribute is needed!
| > | ||
| <FormattedMessage id={`config.popups.${popupTarget}`} /> | ||
| <Icon iconType={popupTarget} /> |
There was a problem hiding this comment.
You have the chat bubble icon in the screenshot and I think I prefer it to this popout icon. We use this popout icon in other places to indicate that something will open in a new window/tab which feels a little inconsistent with the survey iframe behavior.
There was a problem hiding this comment.
I am not sure to understand... It's the same (configured) icon that is also used at the top of itinerary results, at the bottom of an expanded itinerary, and in the app menu. Maybe the a11y text needs to say "Opens in a popup" or something?
There was a problem hiding this comment.
I don't think we need additional text! Using this icon for the feedback button in the app menu makes sense, because in mobile view this will open a brand new window in the browser. But in desktop view, it only opens a modal. And since generally when we use this icon (#859) it means it's opening a new window, it seems more consistent to have a different icon for the desktop nav.
I suppose I don't mind keeping this icon for both, I can defer to the second reviewer! But I do prefer the chat bubble, if that's an option.
There was a problem hiding this comment.
Per our call earlier, I will add additional a11y text to warn of a tab/window opening on mobile. We will keep using any configured icon if any.
There was a problem hiding this comment.
I added the a11y text in 63ba3ff and subsequent commits.
| signIn: Log in | ||
| signOut: Log out |
There was a problem hiding this comment.
Thank you for doing this!
…ith mobile ext window notice.
daniel-heppner-ibigroup
assigned binh-dam-ibigroup and unassigned daniel-heppner-ibigroup
Description
This PR sets the visual content of the "Give Feedback" (or other configured popup) and login buttons to be icons, so that text in some languages doesn't bleed into other content. A popup is also added to these buttons and the language selector. The login button English text is coordinated with the text from the Auth0 login screen.
PR Checklist