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 basic view of navigation bar #265

Merged
merged 2 commits into from
Aug 19, 2019

Conversation

koderjoker
Copy link
Contributor

@koderjoker koderjoker commented Aug 14, 2019

Description
Adds main mobile view of the navigation bar (<770px).

Closes #248

Technical
To view navbar
npm install
npm install -g polymer-cli
cd topnav && polymer serve

Points to be noted for changes

  1. Mobile topnav is not to be made based on our current topnav at archive.org, but rather as an entity of its own on which our new desktop nav is to be based on (due to pre existing accessibility traps, too many DOM elements etc)
  2. Stylesheet (archive.less) is discarded in favour of writing new styles due to its unpredictable effects and to write cleaner and reduce the amount of unused css
  3. Bootstrap is to be no longer used due to the complications it causes in writing one's own styles and existing accessibility issues
  4. Buttons are now used rather than links as per a11y
  5. The traditional ul and li topnav has been replaced in favour of divs in nav in order to tackle styling and alignment issues (margin). No accessibility issues with same.

Existing issues that may need revision

  1. Focus ring is ugly while tabbing. Perhaps due to icon sizing issues, may be resolved when actual icons are in use?

Possible additions

  1. Replace tab focus with lightening the colour of icon's surrounding box while tabbing
  2. Implement above when user hovers over a button for differentiating purposes
  3. Enable focus ring only for keyboard users

Copy link
Contributor

@iisa iisa left a comment

Choose a reason for hiding this comment

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

looks good to me

@iisa iisa merged commit 9afba68 into feature-mobile-topnav Aug 19, 2019
@iisa iisa deleted the 248-implement-main-navigation-bar branch August 19, 2019 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants