feat: add Travel Guide page - #91
Merged
Merged
Conversation
✅ Deploy Preview for devcon-app canceled.
|
✅ Deploy Preview for devconnect-monorepo canceled.
|
✅ Deploy Preview for devcon-monorepo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for devcon-archive canceled.
|
Scott1UP
changed the title
✅ Deploy Preview for devcon-social canceled.
|
✅ Deploy Preview for devconnect-pwa canceled.
|
✅ Deploy Preview for devcon-event-app canceled.
|
didierkrux
approved these changes
Jul 29, 2026
Member
There was a problem hiding this comment.
a few comments:
- maybe add a link to find it in the about menu
- add something about what Aamchi means with an *
- open these 2 links in a new page to avoid loosing the user too quickly
- fix the ratio of the box to the video ratio to avoid black borders
- few mobile issues:
other comments from the AI:
- Contradictory Diwali copy. stay.body says "Devcon overlaps with Diwali... so book your stay early", while culture.diwali_1 says Diwali "falls on November 8, two days after Devcon ends". Devcon 8 is 3 to 6 November 2026 per content/en/intl/home.json, so the culture section is the accurate one. The multi-day festival window arguably does overlap, but as written the two sections read as a factual conflict on a page people will trust for booking decisions.
- Collapsed accordion panels are aria-hidden but still focusable. content-wrap collapses via grid-template-rows: 0fr with overflow: hidden, so links inside a closed panel (hotel CTAs, markdown links) stay in the tab order while hidden from screen readers. That's the aria-hidden-focus violation. Adding inert to the wrapper when closed fixes it cleanly.
- Two snapshot values bypass i18n. TIMEZONE_VALUE and AVG_TEMP_VALUE are hardcoded at index.tsx:119-120 while every neighboring value comes from travel_guide.json. "30–32 °C day / 24–26 °C evening" contains English words that hi and mr will never receive. Moving both into the JSON matches the surrounding pattern.
- Mobile will look broken if this deploys as-is. Only 6 md: and 14 lg: prefixes across 1,020 lines, alongside fixed widths like w-[427px], w-[310px], and h-[430px]. The PR description says tablet and mobile are next, and the missing nav link keeps the blast radius small, so the practical guidance is just not to share the URL until the responsive pass lands.
Scott1UP
changed the title
Collaborator
Author
|
Thanks for review @didierkrux This is just the Desktop version commit — I wanted to set a baseline and push it for an initial review, so thanks for the feedback. For context, I purposefully asked Claude to do the bare minimum for tablet & mobile, as well as ignoring adding it to the navigation for the time being. Will be building and pushing further commits for the remaining functionality today. |
…obile stacking for info tables, and improved video player layout
Collaborator
Author
|
…verage temperature, enhance accessibility with inert attribute in AccordionGroup, and improve link UX with target and rel attributes
didierkrux
approved these changes
Aug 3, 2026
didierkrux
left a comment
didierkrux
left a comment
Member
There was a problem hiding this comment.
latest version is looking great! 👏
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Initial commit for the Travel Guide page:
Future commits will add full device support.