Skip to content

Salimsara/enhancement/556 complete gameart page redesign#584

Merged
Skoivumaki merged 16 commits intodevfrom
salimsara/enhancement/556-complete-gameart-page-redesign-2-2
Dec 4, 2025
Merged

Salimsara/enhancement/556 complete gameart page redesign#584
Skoivumaki merged 16 commits intodevfrom
salimsara/enhancement/556-complete-gameart-page-redesign-2-2

Conversation

@salimsara
Copy link
Contributor

@salimsara salimsara commented Oct 29, 2025

📄 Pull Request Overview

Closes #556

🔧 Changes Made

Implemented the new Figma design to GameArt Page:

  • Created a reusable SectionCard component using the existing ModularCard system
  • Extended ModularCardImageProps to include optional width and height props for better image control in cards

Refactoring/clean up tasks

  • Made the dropdown sidebar visible on mobile screens
  • Adjusted the gap between page title and mobile sidebar.

Checklist Before Submission

  • Functionality: I have tested my code, and it works as expected.
  • JSDoc: I have added or updated JSDoc comments for all relevant code.
  • Debugging: No console.log() or other debugging statements are left.
  • Clean Code: Removed commented-out or unnecessary code.
  • Tests: Added new tests or updated existing ones for the changes made.
  • Documentation: Documentation has been updated (if applicable).

📝 Additional Information

Screenshots:
Screenshot 2025-10-29 at 0 41 04
Screenshot 2025-10-29 at 1 57 35
Screenshot 2025-10-29 at 0 40 32
Screenshot 2025-10-29 at 0 40 23


Known issues:

  • As seen in the screenshots, the diagonal clipping angle varies depending on the aspect ratio of each image (e.g. horizontal vs. vertical, or different heights).
    • Because of this, some cards also end up with extra whitespace between the text section and the image section, especially when the image dimensions differ significantly from the others.
  • The mobile menu does not close automatically when navigating to another page; this issue likely existed before my changes, it is not caused by my code

Copy link
Contributor

@Rutjake Rutjake left a comment

Choose a reason for hiding this comment

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

Good job! The page looks great. However, something is broken in the page navigation: the anchor links do not work (do not scroll to sections) on a mobile device or desktop, and the mobile menu does not close when clicking on a link.


@media (max-width: breakpoint(lg)) {
min-width: 320px;
@media (max-width: 1023px) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the breakpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the breakpoint because I wanted the padding-top to also apply at screen width (1023px), which is exactly between the lg and xl breakpoints. Since that screen width is where sidebar switches to a dropdown, and that creates a gap between the dropdown and the title (which doesn't align with the Figma design). No problem, I will put it back.

@Rutjake
Copy link
Contributor

Rutjake commented Oct 30, 2025

Good job! The page looks great. However, something is broken in the page navigation: the anchor links do not work (do not scroll to sections) on a mobile device or desktop, and the mobile menu does not close when clicking on a link.

Nvm. The sidebar menu issue has been resolved in the dev branch as part of another related issue.

@salimsara salimsara requested a review from Rutjake October 30, 2025 21:58
@Skoivumaki
Copy link
Member

Nvm. The sidebar menu issue has been resolved in the dev branch as part of another related issue.

The problem comes from the removal of WikiContent component, which declares id tags for each section upon rendering. When this branch gets merged, it will break again.
A simple solution would be to include id={section.id} in section mapping in GameArtPage.tsx
Then the legacy html #id based href should work.

Copy link
Member

@Skoivumaki Skoivumaki left a comment

Choose a reason for hiding this comment

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

Good work! Readable and clean code 👍
Theres still small improvements todo:
On tablets the margin/padding between image and text is awkwardly big:
image
See: #584 (comment)

IMO the images look off. I tried to make the images fit more clean to the clip-path frame thing, but honestly no matter how you scale/transform them. They wont look any better. Artist problem not ours... 😅

@salimsara salimsara requested a review from Skoivumaki November 5, 2025 01:09
@Skoivumaki
Copy link
Member

On tablets the margin/padding between image and text is awkwardly big:

Issue still persists

Copy link
Member

@Skoivumaki Skoivumaki left a comment

Choose a reason for hiding this comment

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

Resolved in Discord

@Skoivumaki Skoivumaki merged commit ab1e5c0 into dev Dec 4, 2025
3 checks passed
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.

3 participants