Skip to content

Conversation

@Rutjake
Copy link
Contributor

@Rutjake Rutjake commented Dec 10, 2024

📄 Pull Request Overview

closes 298

🔧 Changes Made

Created flexboxes for the page layout (lng\clans\layout.tsx) and adjusted responsiveness and alignment in the CSS file (new file, ClanPageLayout.scss).


Checklist Before Submission

  • Functionality: I have tested my code, and it works as expected.

Responsiveness worked in the browser and the flexbox size was adjusted according to the page's main menu.

📝 Additional Information

Provide any additional context or information that reviewers may need to know:

  • Screenshots: [Include any screenshots or videos if the changes affect the UI]

Flexbox_Desktop_ScreenShot

Flexbox_Mobile_ScreenShot

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.

Build is failing because of a typo. import './ClanPageLayout.scss'; when it should be import './clanPageLayout.scss'; but change the file name to keep good practice: ClanPageLayout.scss
We dont store CSS inside [lng], code for layout should be stored inside pages layer and should only be imported in [lng]. (see Auth for example: [lng](helper)\auth\layout.tsx) We have a place to store layouts but since this layout is Clan specific I think this will do for now.
image

About FSD folder scheme: https://feature-sliced.design

To summarize:

  • Rename clanPageLayout.scss to ClanPageLayout.scss
  • Move code related to layout to proper location
  • Dont override the old layout yet, since it may break some clan elements.
  • Make sure build and tests pass

@leolabdev
Copy link
Member

leolabdev commented Dec 12, 2024

@Skoivumaki @Rutjake Hey guys, we use CSS (SCSS) modules, not plain CSS.

@Rutjake Rutjake requested a review from Skoivumaki December 17, 2024 10:06
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.

Looks good and works as expected. Only thing missing was the export in index.ts for the layout itself, but I added that myself.

Copy link
Member

@leolabdev leolabdev left a comment

Choose a reason for hiding this comment

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

please use for breakpoints the following below scss function:

image

@Rutjake Rutjake requested a review from Jonroi as a code owner January 7, 2025 13:47
@Rutjake Rutjake requested a review from leolabdev January 7, 2025 13:48
Copy link
Contributor

@Jonroi Jonroi left a comment

Choose a reason for hiding this comment

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

I read the latest comments from the reviewers and noted that the breakpoints have been used correctly and the previous tasks have been completed, so good work

@Jonroi Jonroi requested a review from Skoivumaki January 16, 2025 09:57
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.

same as above

@Skoivumaki Skoivumaki self-assigned this Jan 17, 2025
@Jonroi Jonroi merged commit c543e8a into dev Jan 17, 2025
1 check 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.

5 participants