search

expressjs / expressjs.com

https://expressjs.com
Other
5.23k stars 1.42k forks source link

fix nav links do not reappear after resizing #1502

Closed aastha-cse closed 1 month ago

aastha-cse commented 2 months ago

Fixes #1499

The issue was with the toggling of display style between navmenu and overlay. To solve this I added a new class name 'open' that fixes the problem.

https://github.com/expressjs/expressjs.com/assets/100745494/f874cac4-d955-408c-8624-fc78cf2c327f

crandmck commented 2 months ago

@chrisdel101 Is this affected at all by your recent dark mode changes, or vice-versa?
And do you have any comments in general on these changes, even though they seem quite simple.... since you've been working in the CSS and JS / layout code recently, I thought I'd just check with you.

chrisdel101 commented 2 months ago

So I noticed this issue when I was working on that feature, but I recall I switched back to main (gh-pages branch) and this problem was occurring there as well. So I'm pretty sure I did not cause it. It seemed to be there already. Just wanted to report on this. Will report in on the PR changes soon.

jonchurch commented 2 months ago

clopened to see if it will trigger a build preview

jonchurch commented 2 months ago

I honestly don't know why this works but it does

jonchurch commented 2 months ago

Ahh, the overlay is staying open after resizing the page, leaving an opacity layer over the top of the page

Screenshot 2024-05-03 at 8 12 04 PM

We should find a way to achieve all of this via media queries.

netlify[bot] commented 2 months ago

Deploy Preview for expressjscom-preview ready!

Name Link
Latest commit f0e5c9db90bbb043284eb7a4056dc6121ae3b7b3
Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/663723777beb8200089d4a95
Deploy Preview https://deploy-preview-1502--expressjscom-preview.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

aastha-cse commented 2 months ago

I looked into the overlay issue and fixed it using the same method that i used to fix the nav bar. It was working fine on my local system but I can't figure out the reason of 'deploy fail'.

https://github.com/expressjs/expressjs.com/assets/100745494/b6871ab1-e2a5-4ee1-8702-11894bef6d6d

crandmck commented 1 month ago

The build is failing because Netlify is trying to use Jekyll build dependencies' README files, and at least one README has a malformed liquid directive:

Liquid Exception: Liquid syntax error (line 50): Variable '{{a}' was not properly terminated with regexp: /\}\}/ in node_modules/balanced-match/README.md

This should be fixed by https://github.com/expressjs/expressjs.com/pull/1510 ... @aastha-cse Can you get the changes from that PR so we can preview this one?

chrisdel101 commented 1 month ago

We should find a way to achieve all of this via media queries.

@jonchurch I'm seeing the media queries for the #navmenu . Which parts did you have in mind that could be fixed up?

crandmck commented 1 month ago

This is building now and the preview looks OK to me.... @jonchurch Do we need further changes or can we land this?