Skip to content

CSS Modules Migration for ButtonLink Component#242

Merged
Satoshi-Sh merged 3 commits intomainfrom
refactor/to-scss-ButtonLink
Jul 1, 2025
Merged

CSS Modules Migration for ButtonLink Component#242
Satoshi-Sh merged 3 commits intomainfrom
refactor/to-scss-ButtonLink

Conversation

@Satoshi-Sh
Copy link
Member

@Satoshi-Sh Satoshi-Sh commented Jun 19, 2025

Web Dev Path
238

This PR depends on #239

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

Yes

What is this change?

CSS Modules Migration for ButtonLink Component

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

You could check the styling behavior of Contact Us button on /about page.

Screenshot from 2025-06-19 08-04-07

When should this be merged?

After review and once PR #239 is merged.

@Satoshi-Sh Satoshi-Sh requested a review from a team June 19, 2025 13:17
@Satoshi-Sh Satoshi-Sh self-assigned this Jun 19, 2025
@netlify
Copy link

netlify bot commented Jun 19, 2025

Deploy Preview for webdevpathstage ready!

Name Link
🔨 Latest commit fb57873
🔍 Latest deploy log https://app.netlify.com/projects/webdevpathstage/deploys/685e7f8e31c5bd00089d1eee
😎 Deploy Preview https://deploy-preview-242--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
Member

@Satoshi-Sh could you please pull the latest main and push to your branch?

@Satoshi-Sh Satoshi-Sh force-pushed the refactor/to-scss-ButtonLink branch from 3a3d01f to a6a485e Compare June 23, 2025 11:03
@Satoshi-Sh Satoshi-Sh requested a review from cherylli June 23, 2025 14:48
@Satoshi-Sh Satoshi-Sh force-pushed the refactor/to-scss-ButtonLink branch from 6b1b3ac to 43e974e Compare June 24, 2025 03:50
@Satoshi-Sh Satoshi-Sh requested a review from cherylli June 24, 2025 04:01
Copy link
Member

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

Buttons are not working now, there are a few things need to be done to fix it.

  1. customClassName is always undefined because it was not passed into it (from TwoColumn)
<ButtonLink
  link={link}
  $colorScheme={$btnColorScheme}
  openNewTab={openNewTab}
>
  {linkText}
</ButtonLink>

We need to remove $colorScheme and change it to customClassName
(although I think "customButtonClass" or something like that is more descriptive as it's only used for the buttons)

  1. inverted-grey is now invertedGrey, although maybe we should rename it back to inverted-grey which is more common css naming

$btnColorScheme='inverted-grey' needs to be changed to customBtnClass='invertedGrey' for it to work (but consider changing it back to inverted-grey as mentioned above. In this case, you would only need to change the CSS name in the scss file)

there are also some inverted-white which need to be changed.

  1. this custom class needs to be passed down from about.js

  2. check if other pages is using buttonlink

@Satoshi-Sh Satoshi-Sh force-pushed the refactor/to-scss-ButtonLink branch from 43e974e to 092ffd3 Compare June 24, 2025 13:41
@Satoshi-Sh Satoshi-Sh force-pushed the refactor/to-scss-ButtonLink branch from 092ffd3 to cb905d0 Compare June 24, 2025 14:05
Copy link
Member

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

Buttons are working now, thanks!

@Satoshi-Sh
Copy link
Member Author

Thanks for reviewing this in detail. @cherylli

Copy link
Member

@briangesteban briangesteban left a comment

Choose a reason for hiding this comment

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

Fix Request

  • Need to fix buttons on the Homepage as well. The background color and transition is not working properly.

Improvement Suggestion

  const buttonClass = customBtnClass
    ? `${styles.buttonLink} ${styles[customBtnClass]}`
    : styles.buttonLink;

This code snippet above seems to be re-usable, and might be use in the SubmitButton component. We could create a helper function under utils for this, something like getButtonStyle or something similar.

@Satoshi-Sh
Copy link
Member Author

Thanks for the review, @briangesteban .
I also needed to update the prop name of TwoColumn component on the blog page.

const buttonClass = customBtnClass
? ${styles.buttonLink} ${styles[customBtnClass]}
: styles.buttonLink;

I keep this for now as submission button doesn't have the effect.

@Satoshi-Sh Satoshi-Sh requested a review from briangesteban June 27, 2025 11:44
Copy link
Member

@briangesteban briangesteban 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 to me now. Everything's working back smoothly. Thanks @Satoshi-Sh !

Copy link
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.

ButtonLinks all looks good. Thanks @Satoshi-Sh!

@Satoshi-Sh Satoshi-Sh merged commit e47c876 into main Jul 1, 2025
4 checks passed
@Satoshi-Sh Satoshi-Sh deleted the refactor/to-scss-ButtonLink branch July 1, 2025 01:09
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