expressjs.com icon indicating copy to clipboard operation
expressjs.com copied to clipboard

fix nav links do not reappear after resizing

Open aastha-cse opened this issue 1 year ago • 7 comments

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

aastha-cse avatar Apr 29 '24 10:04 aastha-cse

@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.

crandmck avatar May 02 '24 04:05 crandmck

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.

chrisdel101 avatar May 03 '24 21:05 chrisdel101

clopened to see if it will trigger a build preview

jonchurch avatar May 03 '24 23:05 jonchurch

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

jonchurch avatar May 04 '24 00:05 jonchurch

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.

jonchurch avatar May 04 '24 00:05 jonchurch

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.

netlify[bot] avatar May 04 '24 06:05 netlify[bot]

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

aastha-cse avatar May 04 '24 06:05 aastha-cse

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?

crandmck avatar May 04 '24 17:05 crandmck

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?

chrisdel101 avatar May 04 '24 21:05 chrisdel101

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

crandmck avatar May 07 '24 02:05 crandmck