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

Update primary nagivation dropdown UX #3769

Merged
merged 7 commits into from
Feb 22, 2024
Merged

Update primary nagivation dropdown UX #3769

merged 7 commits into from
Feb 22, 2024

Conversation

theskumar
Copy link
Member

@theskumar theskumar commented Feb 12, 2024

Description

  • Fix the dropdown being hidden on the beta all page
  • Use mouseover for quick access
  • Update over states
  • Update the urls to that any external urls can also be added via
    settings, removes dependeny of reverse while specifying urls
  • How active page in the dropdown as well, a dark-blue border
  • Update the focus outline to be a simple dashed-border, instead of bulky outline

Test Steps

  • Checkout the primary navigation in the top-header

- Use mouseover for quick access
- Update over states
- Update the urls to that any external urls can also be added via
  settings, removes dependeny of `reverse` while specifying urls
- How active page in the dropdown as well, a dark-blue border
@theskumar theskumar self-assigned this Feb 12, 2024
@theskumar theskumar added the Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). label Feb 12, 2024
@frjo
Copy link
Member

frjo commented Feb 12, 2024

Nice works! Open on hover, or not, is a personal preference. I will ask the other maintainer in the meeting tomorrow what they prefer.

Two minor items that would be nice to resolve:

  1. The dotted outline looks a bit off I think. I see it uses focus-visible so it should only show for keyboard navigation. But I still see it using only the mouse/trackpad.
  2. When you hover over the longes menu item the bold text makes the with of the menu increase a tiny bit.
Skärmavbild 2024-02-12 kl  13 07 59

x-show="open"
x-transition
@click.outside="open = false"
class="min-w-48 absolute block bg-white shadow-xl z-20 mt-4 border rounded-sm"
Copy link
Member

Choose a reason for hiding this comment

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

min-w-48 seems to not generate any styles? Why not use min-w-max? Having menu item break on two lines is undesirable.

Copy link
Member Author

Choose a reason for hiding this comment

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

min-w-48 ensure the width of two dropdown are similar. It won't break them in two lines as it's not setting any max width. That said, I should consider added generous max-width so the dropdown doesn't look weird for a link with too many words.

Let me know if it all sounds reasonable?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is could be b/w you are not using the latest tailwind, which introduced min-w-*

@theskumar
Copy link
Member Author

Nice works! Open on hover, or not, is a personal preference. I will ask the other maintainer in the meeting tomorrow what they prefer.

Two minor items that would be nice to resolve:

  1. The dotted outline looks a bit off I think. I see it uses focus-visible so it should only show for keyboard navigation. But I still see it using only the mouse/trackpad.
  2. When you hover over the longes menu item the bold text makes the with of the menu increase a tiny bit.
Skärmavbild 2024-02-12 kl 13 07 59

Which browser are you using, this is what i see:

Screenshot 2024-02-12 at 12  33 18@2x

@theskumar
Copy link
Member Author

The visible-focus seems to be not working with x-trap and mouseover, already spent sometime trying to figure out a solution, but no luck. I'll give it another try.

@theskumar theskumar requested a review from frjo February 12, 2024 13:05
@theskumar
Copy link
Member Author

@frjo any feedback on the nav getting opened while on hover? The outline issue is only happening if the mousehover is used. If mouseover is not desired, there is need to chase after the focus issue.

@theskumar
Copy link
Member Author

Update and fixed the outline issue on safari, it should look better now.

Screenshot 2024-02-15 at 10  01 57@2x

@frjo frjo added Status: RTBC Internal Dev use only Type: Minor Minor change, used in release drafter Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team labels Feb 15, 2024
@frjo
Copy link
Member

frjo commented Feb 16, 2024

Latest version on test now.

@wes-otf
Copy link
Contributor

wes-otf commented Feb 16, 2024

Hey! Added functionality looks good to me, I'm just a little confused about what purpose the dotted border serves? I didn't see it changing on test at all, just stayed on All Submissions/All Projects. On Firefox 121 I can't seem to use keyboard nav at all with Hypha if that's what this is in reference to?

@frjo frjo removed the Status: RTBC Internal Dev use only label Feb 21, 2024
@frjo frjo merged commit 06e53e6 into main Feb 22, 2024
10 checks passed
@frjo frjo added Type: Patch Mini change, used in release drafter and removed Type: Minor Minor change, used in release drafter labels Feb 27, 2024
@theskumar theskumar deleted the fix/primary-nav-ux branch March 9, 2024 06:14
wes-otf pushed a commit that referenced this pull request May 7, 2024
- Fix the dropdown being hidden on the beta all page
- Use mouseover for quick access
- Update over states
- Update the urls to that any external urls can also be added via
  settings, removes dependeny of `reverse` while specifying urls
- How active page in the dropdown as well, a dark-blue border
- Update the focus outline to be a simple dashed-border, instead of
bulky outline
wes-otf pushed a commit that referenced this pull request May 8, 2024
- Fix the dropdown being hidden on the beta all page
- Use mouseover for quick access
- Update over states
- Update the urls to that any external urls can also be added via
  settings, removes dependeny of `reverse` while specifying urls
- How active page in the dropdown as well, a dark-blue border
- Update the focus outline to be a simple dashed-border, instead of
bulky outline
Vldln pushed a commit to equalitie/hypha that referenced this pull request May 28, 2024
- Fix the dropdown being hidden on the beta all page
- Use mouseover for quick access
- Update over states
- Update the urls to that any external urls can also be added via
  settings, removes dependeny of `reverse` while specifying urls
- How active page in the dropdown as well, a dark-blue border
- Update the focus outline to be a simple dashed-border, instead of
bulky outline
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants