django / djangoproject.com

Source code to djangoproject.com
https://www.djangoproject.com/
BSD 3-Clause "New" or "Revised" License
1.86k stars 938 forks source link

Fixed #1435 -- hamburger tab key focus issue fixed #1451

Closed Bhagyarsh closed 2 weeks ago

Bhagyarsh commented 6 months ago

Issue #1435

Screenshots for reference Before: before#1435

After: after#1435

Bhagyarsh commented 6 months ago

Hi Sanyam,

Thanks for looking into it! I tested on the same Chrome version (120.0.6099.109-1)on Linux and Firefox (120.0.1-1). I also cleared my cache, and everything looks good on my end.

Could it be a cache thing on your side? Give it a shot after clearing and let me know if it's still acting up.

image

Bhagyarsh commented 2 months ago

Hi @CuriousLearner , I was wondering if you had a chance to review it further. Are there any changes required before merging it? I'm happy to make any adjustments based on your feedback. Please let me know if you have any questions. Thanks for your time, and I look forward to your response. Best regards,

bmispelon commented 2 months ago

Hi @Bhagyarsh and thanks for this PR!

I took a look a it on my local version (using firefox) and your approach does seem to work (I did have to run make compile-scss after switching branches, maybe that's why the other commenter got the wrong behavior?).

However I'm a bit hesitant about adding more complexity to the HTML. Instead I tried an alternative approach and used flex for the layout of the whole header instead of floats. This lets the browser naturally focus things without having to add any extra HTML. The result is in this PR: https://github.com/django/djangoproject.com/pull/1523 Can you tell me what you think?

Thanks! ✨

CuriousLearner commented 2 months ago

Hi @Bhagyarsh

Sorry, I didn't get a notification for your earlier comment.

I'll take a look at this soon.

bmispelon commented 2 weeks ago

Hi, I ended up merging this feature in #1523. Thanks for being part of this team effort and working on improving the website 🚀