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.

Sign up for GitHub

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

Jump to bottom

Mobile nav: main menu fixes #320

Merged
merged 9 commits into from
Oct 10, 2019
Merged

Mobile nav: main menu fixes #320

merged 9 commits into from
Oct 10, 2019

Conversation

iisa
Copy link
Contributor

@iisa iisa commented Oct 8, 2019 •
edited
Loading

Description

These are behavioral fixes to the mobile nav's media menu

#316 - label can be clicked to open up slide out menu
#317 - while slide out menu is open, clicking on next icon changes tab and keeps menu open
#318 - when slide out menu is open and media menu is closed, slide out menu also closes

*** NOTE: this also includes the directory change, so base is pointed to that to only show changes. On approval, this must be merged AFTER #311 is merged

Evidence

ezgif com-video-to-gif

Internet Archive added 3 commits October 8, 2019 09:52
Media menu: simplify DOM & style so icon and label are clickable
4850896 
Fixes issue #316

Media menu linting fix - no nested ternaries
Media menu: clicking on multiple media icons keeps secondary menu ope…
eaf533b 
…n, tabs change accordingly

Issue #317

+ fixed ternary linting issue

Next step: add support for menu context changes
Media menu: close open slide out menu when media menu closes
9d50bd9 
Issue: #318
shaneriley
shaneriley reviewed Oct 9, 2019
View reviewed changes
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
packages/mobile-top-nav/src/media-menu.js Outdated Show resolved Hide resolved
@iisa iisa changed the base branch from start-package-dir to feature-mobile-topnav October 10, 2019 05:41
Simplify DOM & CSS
7e1998d 
- use only elements that we absolutely need
- simplify element animation to mainly use css transitions
@iisa iisa requested a review from shaneriley October 10, 2019 10:17
Internet Archive added 3 commits October 10, 2019 10:11
jbuckner
jbuckner approved these changes Oct 10, 2019
View reviewed changes
Copy link
Contributor

@jbuckner jbuckner left a comment

Choose a reason for hiding this comment

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

🚀

@iisa iisa merged commit 2596240 into feature-mobile-topnav Oct 10, 2019
@iisa iisa deleted the main-menu-fixes branch October 10, 2019 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbuckner jbuckner jbuckner approved these changes

@koderjoker koderjoker Awaiting requested review from koderjoker

@shaneriley shaneriley Awaiting requested review from shaneriley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iisa @shaneriley @jbuckner