[CAN-143] Merge Mobile Drawer into Develop #120
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem - Mobile nav switch from overlay to Drawer Solution - Created an animated drawer - drawer.jsx - Include react-transition-group - Removed overlay *just commented out* Note - Need to implment links - Blurred background - Mobile Nav Links
Problem - usePreventScrolling wasn't being returned on unmount due to RTG Solution - make the usePreventScrolling hook accept a conditional variable - This way, you can dynamically prevent scrolling Note - RTG ~ React-Transition-Group - Even when unmounting, be careful of unmount calls, they aren't handled by Next or React anymore!
Problem - Links have not been added Solution - Added links for mobile nav using previous mobile nav popup - Changed Color scheme to dark - can be easily changed back Note
Problem - Background wanted to be coloured / faded - Weird when component dismounts as background slides with it Solution - Add a nested animation to the actual drawer - Add animation delays for both background and drawer Note
Problem - Animation slide in at wrong side - Active page not highlighted in nav Solution - Change react-transition-group from -100% to calc(100vw - 100%) - Implement isHomepage logic copied from Navbar Links Component and add to active link Note
Problem - Failed to push due to failing tests - This is because Overlaynav has been massively changed. Solution - Mock Drawer Component and remove close mock Note
Problem - Position of language switcher felt wrong in drawer Solution - Suggested a new position for mobile/tablet Note - Just a suggestion, happy to go for something else as well!
Problem - Giving width-100 to the outer div of the language switcher Solution - Take it out, fixes desktop navigation Note - Really not sure why this broke the navigation, but fixed
There was a problem hiding this comment.
Going to do a Comment and not an Approve just to remind myself to review the new test suite when it's pushed as well as the few other open points. Definitely agree this is a version increase, and think because it's a whole new feature it's definitely a minor version increase. So, to cut a long story short. VERY EXCITED FOR CCB 1.1.0 😸
Very nice work on the drawer, actually feels like such an intuitive experience!!! Made a minor change regarding the position of the language switcher, happy to discuss this though! Will also have a crack at the bug you have (will stay away from tests because pretty sure you've already been working on that @fabie37 !! And btw, if anyone is reading this, it's his birthday, so show him some love!! ❤️ 🥼 🧑🔬
added 4 commits
September 12, 2024 19:02
Problem - Were only calling onClick when pathname was home pathname Solution - If onClick provided, call it for all paths Note - I may have misunderstood, but it is supposed to do nav right? Note2 - Removed outdated test which checked onClick only called as such
Problem - Want to build and test for MRs to check we good Solution - Added workflow, need to trigger it now
Problem - Pipeline can't run if it's only on main at the moment Solution - Add Workflow here already (might have to be on develop)
Problem - Some outdated casing for test module imports Solution - Fix them
fabie37
commented
Sep 16, 2024
Problem - Unable to get to club listing from clicking on image Solution - Wrap image in a tag with appropriate class names for css grid. Note
New Features: - Animated Mobile Nav Drawer - Clickable Popup Image Tag - Drawer Component Test Suite Note - N/A
Problem
- Language Switcher on drawer nav not positioned properly
Solution
- Adjust height and width of container to be in 100%
- Change position from relative to absolute of parent
- Switch Language Switcher to display none instead of invisible
- Removes left margin noticed on Language switcher element
Note
marcaufderheyde
approved these changes
Sep 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
Change mobile nav overlay to a sliding drawer.
New Dependencies:
react-group-transitions
Features:
Tasks: