Skip to content

fix initial render for vertical#264

Merged
adhi-thirumala merged 5 commits intomainfrom
fix-initial-vert-render
Jan 22, 2026
Merged

fix initial render for vertical#264
adhi-thirumala merged 5 commits intomainfrom
fix-initial-vert-render

Conversation

@jlevine18
Copy link
Contributor

@jlevine18 jlevine18 commented Jan 22, 2026

Summary by CodeRabbit

  • Improvements

    • Map display is now responsive: imagery and layout adapt to portrait and landscape for clearer visuals on phones and tablets.
    • Presentation moved to CSS-driven styling for consistent alignment, spacing, and cleaner markup for better accessibility.
  • New Content

    • Added a new organization entry ("124h") and updated assignment listings to include it.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Walkthrough

Adds a .mapImage CSS rule and a portrait-orientation media query to make the map image full-width, block, and centered. Replaces the previous conditional <img> in the open-house page with a <button> containing a <picture>: a portrait <source> for /oh_map_vertical.svg and a fallback <img src="/oh_map.svg"> using the .mapImage class; click handler and aria-label are unchanged. Updates data: replaces token "sail" with "124h" in assignments_config.json and adds a new 124h org entry to orgs_config.json. No exported/public signatures modified.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix initial render for vertical' is somewhat vague and doesn't clearly convey what the actual change is—whether 'vertical' refers to orientation, a component, or another concept. Consider a more descriptive title such as 'Replace img with picture element for responsive venue map' or 'Use CSS for map styling to support portrait orientation'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Jan 22, 2026

Deploying with Cloudflare Pages

Name Result
Last commit: e4b40a2
Preview URL: https://a690f104.acmuiuc.pages.dev
Branch Preview URL: https://fix-initial-vert-render.acmuiuc.pages.dev

@jlevine18 jlevine18 marked this pull request as ready for review January 22, 2026 00:41
@jlevine18 jlevine18 requested review from a team as code owners January 22, 2026 00:41
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/app/`(open-house)/open-house/page.tsx:
- Around line 400-423: Replace the two <img> tags inside the button (currently
using styles.horizontalMap and styles.verticalMap) with a single <picture> that
contains two <source> elements using media queries (e.g. media="(orientation:
landscape)" srcSet="/oh_map.svg" and media="(orientation: portrait)"
srcSet="/oh_map_vertical.svg") and a single fallback <img> with the common alt
and the appropriate className (or a shared class) so only the matching asset is
downloaded; keep the button, its type, aria-label and onClick handler
(handleClick) unchanged and preserve accessibility attributes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/app/`(open-house)/open-house/page.tsx:
- Around line 413-419: The CSS module is missing the mapImage class used by the
<img> (styles.mapImage) in the Open House page, causing layout and marker
positioning to break; add a .mapImage rule to the page module CSS that enforces
proper sizing and layout (e.g., width: 100% and display: block or other
responsive sizing you use) so the image fills its container and maintains
coordinate mapping for booth markers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/app/`(open-house)/open-house/page.module.css:
- Around line 113-124: The portrait media query repeats properties already set
on .mapImage; open the `@media` (orientation: portrait) block and remove the
redundant width: 100% and display: block declarations so that only margin: 0
auto remains for .mapImage, leaving the base .mapImage rule to provide width and
display via cascade.

@github-actions
Copy link

😢 Cloudflare Pages - Deployment failed
Please check the workflow logs for details.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/app/`(open-house)/data/orgs_config.json:
- Around line 444-447: The JSON file ends with a trailing comma after the final
object (the block containing { "text": "Website", "link":
"https://honors.cs124.org" }) which causes a parse error; remove the comma
immediately following the closing brace of the outer object so the JSON is valid
(ensure the final ']' and closing '}' are not followed by a comma).
♻️ Duplicate comments (1)
src/app/(open-house)/open-house/page.module.css (1)

119-125: Redundant property declarations in the portrait media query.

The width: 100% and display: block properties in the media query are already defined in the base .mapImage class and will cascade. Only margin: 0 auto is needed here.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@jlevine18
Copy link
Contributor Author

@adhi-thirumala

Copy link
Contributor

@adhi-thirumala adhi-thirumala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems legit

@adhi-thirumala adhi-thirumala merged commit 201fe88 into main Jan 22, 2026
5 checks passed
@adhi-thirumala adhi-thirumala deleted the fix-initial-vert-render branch January 22, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants