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

Fix nav menu #297

Merged
merged 10 commits into from
Dec 9, 2024
Merged

Fix nav menu #297

merged 10 commits into from
Dec 9, 2024

Conversation

DinneK
Copy link
Contributor

@DinneK DinneK commented Dec 9, 2024

module-name: Fix navigation for metrics

Problem

The menu in the navigation bar does not open and close correctly.

Solution

Change logic in nav.js to account for open/close. Change css to match 3.0 design.

Result

Menu works as intended.

Test Plan

Test in browser locally and in prod.

Bug

When hovering over menu a text-decoration: underline still shows up.

Next Steps

Fix line under menu SVG to none.

Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
Signed-off-by: Dinne Kopelevich <[email protected]>
@DinneK DinneK added enhancement New feature or request front-end javascript Pull requests that update Javascript code metrics labels Dec 9, 2024
@DinneK DinneK self-assigned this Dec 9, 2024
Copy link
Collaborator

@natalialuzuriaga natalialuzuriaga left a comment

Choose a reason for hiding this comment

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

The nav menu styling looks and works great!! There is a nitpick regarding the toggling but ultimately this isn't blocking anything and can be addressed at a later time.

app/src/js/nav.js Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

prettier

[prettier] reported by reviewdog 🐶

const range = [];
const start = Math.max(2, currentPage - visibleRange);
const end = Math.min(totalPages - 1, currentPage + visibleRange);


[prettier] reported by reviewdog 🐶

range.push(1);


[prettier] reported by reviewdog 🐶

if (start > 2) range.push('...');


[prettier] reported by reviewdog 🐶

range.push(i);


[prettier] reported by reviewdog 🐶

if (end < totalPages - 1) range.push('...');


[prettier] reported by reviewdog 🐶

if (totalPages > 1) range.push(totalPages);


[prettier] reported by reviewdog 🐶

return range;

app/site/_layouts/org-report.liquid Show resolved Hide resolved
app/site/_layouts/org-report.liquid Show resolved Hide resolved
app/site/_layouts/org-report.liquid Show resolved Hide resolved
app/src/css/style.css Show resolved Hide resolved
app/src/css/style.css Show resolved Hide resolved
app/src/js/projects.js Show resolved Hide resolved
app/src/js/projects.js Show resolved Hide resolved
app/src/js/projects.js Show resolved Hide resolved
app/src/js/projects.js Show resolved Hide resolved
app/src/js/projects.js Show resolved Hide resolved
@DinneK DinneK merged commit b2a5b5a into dev Dec 9, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request front-end javascript Pull requests that update Javascript code metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants