Skip to content

Refactor/to scss layout#273

Merged
Satoshi-Sh merged 6 commits intomainfrom
refactor/to-scss-layout
Aug 12, 2025
Merged

Refactor/to scss layout#273
Satoshi-Sh merged 6 commits intomainfrom
refactor/to-scss-layout

Conversation

@Satoshi-Sh
Copy link
Copy Markdown
Member

Web Dev Path
238

Have you updated the CHANGELOG.md file? If not, please do it.

Yes

What is this change?

CSS modules migration for Layout component

If necessary, please describe how to test the new feature or fix.

Please check the main styling on any page.

When should this be merged?

After reviews

@Satoshi-Sh Satoshi-Sh requested a review from a team August 9, 2025 10:39
@Satoshi-Sh Satoshi-Sh self-assigned this Aug 9, 2025
@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 9, 2025

Deploy Preview for webdevpathstage ready!

Name Link
🔨 Latest commit 4c669a6
🔍 Latest deploy log https://app.netlify.com/projects/webdevpathstage/deploys/689b26182bea38000815ed60
😎 Deploy Preview https://deploy-preview-273--webdevpathstage.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@cherylli
Copy link
Copy Markdown
Member

cherylli commented Aug 9, 2025

can we delete the styles.js file in the same folder now?

@Satoshi-Sh
Copy link
Copy Markdown
Member Author

can we delete the styles.js file in the same folder now?

Thanks. Deleted.

Copy link
Copy Markdown
Member

@oluwatobiss oluwatobiss left a comment

Choose a reason for hiding this comment

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

Do we need this rule at all? It looks to be doing nothing.

@Satoshi-Sh
Copy link
Copy Markdown
Member Author

Do we need this rule at all? It looks to be doing nothing.

I didn't notice this. I also checked the styling without the rules, and the web site seems the same.
We could delete the class, but how can we make sure the rules not affecting anything?

@oluwatobiss

@oluwatobiss
Copy link
Copy Markdown
Member

We could delete the class, but how can we make sure the rules not affecting anything?

It has no impact on all the pages I’ve checked. However, even if it does, it seems odd to change the stacking context of the website’s main container. It can make debugging challenging. This case of uncertainty about whether the rule has any side effects is a good example of the unintended consequences that such implementation can cause. Since we are working on the style conversion, it may be best to delete it now so we can determine its impact as we progress.

Copy link
Copy Markdown
Member

@oluwatobiss oluwatobiss left a comment

Choose a reason for hiding this comment

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

All looks good. Thanks @Satoshi-Sh!

@Satoshi-Sh Satoshi-Sh merged commit 6163eb3 into main Aug 12, 2025
3 of 4 checks passed
@Satoshi-Sh Satoshi-Sh deleted the refactor/to-scss-layout branch August 12, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants