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

Add dropdowns to navigation and access nav items via file #3679

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

sandeepsajan0
Copy link
Member

Fixes #3670

  • Added a code setting file to define the nav item.
  • Use the dropdown to show nav item categories.
  • Add user role permissions to nav items and categories.
  • Make dropdowns accessible.

image

@click.prevent="open = ! open">
{{ item.title }}
</a>
<div x-cloak x-show="open" x-transition @click.outside="open = false" class="absolute block bg-white min-w-max shadow-lg z-1 mt-7 ">
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add x-trap="open" and perhaps also x-on:keydown.escape.window="open = false".

The first will trap focus inside the drop down and the other allow "esc" to close the dropdown.

},
],
},
]
Copy link
Member

Choose a reason for hiding this comment

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

Could we here check if there is a setting that overrides the "categories" for submissions and project menu items?

Something like:

if settings.MENU_SUBMISSIONS_SUBITEMS:
    nav_items[1]["categories"] = settings.MENU_SUBMISSIONS_SUBITEMS

Or are there better solutions? I want it to be easy for a developer to add/change/maintain the menu subitems.

Copy link
Member Author

Choose a reason for hiding this comment

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

@frjo Now it can be easily managed via settings. We just have to pass a compact JSON in the env variable.
Like: APPLY_NAV_SUBMISSIONS_ITEMS='[{"title":"All Submissions","url":"apply:submissions:list","permission_method":"is_apply_staff_or_reviewer_required"},{"title":"Staff Assignments","url":"apply:submissions:staff_assignments","permission_method":"is_apply_staff"},{"title":"Reviews","url":"apply:submissions:reviewer_leaderboard","permission_method":"is_apply_staff"},{"title":"Results","url":"apply:submissions:result","permission_method":"is_apply_staff"}]'

@sandeepsajan0 sandeepsajan0 force-pushed the feature/gh-3670-navigations branch from 47e9597 to c1275a6 Compare January 9, 2024 06:50
@frjo frjo self-requested a review January 9, 2024 11:04
@frjo
Copy link
Member

frjo commented Jan 9, 2024

Works really well. Some minor issues I found.

Skärmavbild 2024-01-09 kl  11 49 20

  1. A small black triangle or similar on items with a drop down is a nice indication to the users I think.
  2. The selects have z-index: 99995; // to override any modals set to come out on top.
  3. I think we can remove this from all the views since it is now part of the drop down.
  4. The blue focus ring on "All submissions" does not look so nice. Focus indications are important for accessibility but can we use the recent :focus-visible instead of :focus? That way focus only show up when needed (keyboard navigation etc.) https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible

@frjo frjo added Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter Status: Needs testing Tickets that need testing/qa labels Jan 10, 2024
@wes-otf wes-otf added Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team and removed Status: Needs testing Tickets that need testing/qa labels Jan 24, 2024
@wes-otf
Copy link
Contributor

wes-otf commented Jan 26, 2024

@sandeepsajan0 the testers said that all looked good besides z-index, the menus were still getting caught behind items with a higher index:
image

all good otherwise!

@frjo frjo removed the Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team label Jan 29, 2024
@sandeepsajan0 sandeepsajan0 force-pushed the feature/gh-3670-navigations branch from d22bd51 to 9668b93 Compare February 8, 2024 08:42
@@ -7,7 +7,7 @@
right: 0;
bottom: 0;
left: 0;
z-index: 20;
z-index: 9;
Copy link
Member Author

Choose a reason for hiding this comment

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

@frjo I have fixed the nav dropdown stacking issue here but just not sure if it is messing with any other UI component.
image

@frjo frjo added the Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team label Feb 8, 2024
@frjo
Copy link
Member

frjo commented Feb 8, 2024

@sandeepsajan0 Excellent work on fixing the z-index issue!

@wes-otf wes-otf added Status: Tested - approved for live ✅ and removed Status: Needs user testing 👷 Tasks that should be tested by OTF's user test team labels Feb 8, 2024
@frjo
Copy link
Member

frjo commented Feb 8, 2024

@sandeepsajan0 Item 4 (focus outline) from #3679 (comment), did you not fix that at one point? I see it again now.

Very minor issue that we can fix at a later stage as well.

@frjo
Copy link
Member

frjo commented Feb 8, 2024

@sandeepsajan0 Added a commit that makes the new drop down menu active on front page as well. Looks ok?

@frjo frjo merged commit 2d66bf1 into main Feb 12, 2024
10 checks passed
@sandeepsajan0
Copy link
Member Author

Yes, it looks good. It should be there.
I added focus-visible but I'll check again for its outline.

@theskumar theskumar deleted the feature/gh-3670-navigations branch February 12, 2024 10:39
wes-otf pushed a commit that referenced this pull request May 7, 2024
Fixes #3670 

- Added a code setting file to define the nav item.
- Use the dropdown to show nav item categories.
- Add user role permissions to nav items and categories.
- Make dropdowns accessible.


Co-authored-by: Fredrik Jonsson <[email protected]>
wes-otf pushed a commit that referenced this pull request May 8, 2024
Fixes #3670 

- Added a code setting file to define the nav item.
- Use the dropdown to show nav item categories.
- Add user role permissions to nav items and categories.
- Make dropdowns accessible.


Co-authored-by: Fredrik Jonsson <[email protected]>
Vldln pushed a commit to equalitie/hypha that referenced this pull request May 28, 2024
)

Fixes HyphaApp#3670 

- Added a code setting file to define the nav item.
- Use the dropdown to show nav item categories.
- Add user role permissions to nav items and categories.
- Make dropdowns accessible.


Co-authored-by: Fredrik Jonsson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Tested - approved for live ✅ Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Navigations(dropdown for submissions and projects)
3 participants