Refactor/to-scss-blog-post-container

[briangesteban]

Web Dev Path
238

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

Yes

What is this change?

• Remove theming from styled-components in BlogPostContainer.

Were there any complications while making this change?

None

How to replicate the issue?

N/A

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

N/A

When should this be merged?

After the reviews.

Image Reference

[netlify]

✅ Deploy Preview for webdevpathstage ready!

Name	Link
🔨 Latest commit	16de68f
🔍 Latest deploy log	https://app.netlify.com/projects/webdevpathstage/deploys/688e60fa0a6f910008a600f8
😎 Deploy Preview	https://deploy-preview-248--webdevpathstage.netlify.app
📱 Preview on mobile	Toggle 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]

how can theming get rid of a prop to pass in a custom color, which could be different from either (or any) theme values?

Although for this particular component, we could just remove bgColor and $color props are they are same as the default for TwoColumn

[briangesteban]

how can theming get rid of a prop to pass in a custom color, which could be different from either (or any) theme values?

Although for this particular component, we could just remove bgColor and $color props are they are same as the default for TwoColumn

For the theming(ThemeProvider) to work, we should have color stylings live in the stylesheet, by using the theme tokens (CSS Variables) applied to component/elements styles.

For passing custom color/style issue, we could instead create a helper function or enums to have restricted options of class names to be pass down to that component. We could create many custom classes as we want since it will all live on the stylesheets and won't be on our way on the component logic. Yes, we're still using props to be able to give it a custom color/style, but we've lessen it to one prop and only using html/js logic by creating a helper function that returns a pre-defined classNames. All stylings properties/values are still inside the stylesheet, which won't break the theming.

Here's an example of what we could possibly do:

Helper function to return a defined className for TwoColumns:

// Could be a Switch case if there's a lot to define
const getStylesTwoColumns = (style) => {
if(style === "tc-red") return ".two-columns--red";
else if(style === "tc-blue" return ".two-columns--blue";
else return ".tc-default";
}

Style sheet for TwoColumns:

@use "./colors" as *;

.twoColumns {
someProps: someValues;
}

// Defined custom stylings
.tc-red {
 background-color: var(--color-red);
}

.tc-blue {
background-color: var(--color-blue);
}

.tc-default {
background-color var(--color-white);
border-radius: 2rem;
width: 50%;
}

Assuming TwoColumns component have something like this:

const TowColumns = ({someProp, anotherProp, customStyle = "tc-default", children}) => {
return (
 <section className={[styles.twoColumns, customStyle ]}>
    <h2>{someProp}</h2>
    <p>{anotherProp}</p>
    {children}
 </section>
)
}

Implementation of the helper function to assign the custom class:

<TwoColumns someProp={heading} anotherProp={content} style={getStyles("tc-blue")}
  {children}
</TwoColumns>

[cherylli]

I suggest for this component, we remove the props and use the default, as the props are just the defaults.

We should proceed on the migration task same as the other components, using css variables.

We haven't actually have a plan to include another theme. And honestly I don't see having themes is normal for a "company/organisation website" in oppose to a website people uses like github/jira.

We are complicating this task by trying to fit into an unplanned feature. If we do ever implement theming, I suggest we can look at the Theme section in the CSS Modules repo. https://github.com/css-modules/css-modules/blob/master/docs/theming.md

This looks a lot more straightforward.

What do you think @mariana-caldas

[mariana-caldas]

I suggest for this component, we remove the props and use the default, as the props are just the defaults.

We should proceed on the migration task same as the other components, using css variables.

We haven't actually have a plan to include another theme. And honestly I don't see having themes is normal for a "company/organisation website" in oppose to a website people uses like github/jira.

We are complicating this task by trying to fit into an unplanned feature. If we do ever implement theming, I suggest we can look at the Theme section in the CSS Modules repo. https://github.com/css-modules/css-modules/blob/master/docs/theming.md

This looks a lot more straightforward.

What do you think @mariana-caldas

Yes, let's keep focused on the scope. It's good to know CSS modules offer theming, though. Perhaps we could create a "skeleton" themed template that would serve as a baseline for future projects. Thanks @cherylli and @briangesteban !

[briangesteban]

Unrelated to this change
This should be Square Brackets altTag='Hashtag'

I've updated the altTag to an empty string instead, since the image is decorative. This will enhance accessibility and comply with W3C standards regarding decorative images.

Reference: W3C Standards on Decorative Images

Cheryl:
I suggest for this component, we remove the props and use the default, as the props are just the defaults.

We should proceed on the migration task same as the other components, using css variables.

We haven't actually have a plan to include another theme. And honestly I don't see having themes is normal for a "company/organisation website" in oppose to a website people uses like github/jira.

We are complicating this task by trying to fit into an unplanned feature. If we do ever implement theming, I suggest we can look at the Theme section in the CSS Modules repo. https://github.com/css-modules/css-modules/blob/master/docs/theming.md

This looks a lot more straightforward.

Mariana:
Yes, let's keep focused on the scope. It's good to know CSS modules offer theming, though. Perhaps we could create a "skeleton" themed template that would serve as a baseline for future projects.

These are noted. I've removed the color styling props and let the default take over for this component.

Thanks for the review @cherylli and @mariana-caldas !

[Satoshi-Sh]

Looks good to me, and thanks for the reference for altTag.

image='/images/svg/square-brackets.svg' // Decorative Image
altTag='' // Intentionally left empty to comply with W3C accessibility standards.

You could delete the comments on line 27/28

You can also update other TwoColumn's altTag in this PR.

[cherylli]

Looks good to me, and thanks for the reference for altTag.

image='/images/svg/square-brackets.svg' // Decorative Image
altTag='' // Intentionally left empty to comply with W3C accessibility standards.

You could delete the comments on line 27/28

You can also update other TwoColumn's altTag in this PR.

I agree these comments can be removed, and there are still conflicts need to be resolved

[cherylli]

please see comment here, just want to mark as "requested changes" to track status
#248 (comment)

[briangesteban]

I've resolved the conflicts, removed the comments, and updated the other altTags from TwoColumn components to comply with the W3C standards.

If I've missed something, please let me know.

Thank you @cherylli and @Satoshi-Sh for the review!

[cherylli]

looks good. Thanks!

[Satoshi-Sh]

Thanks for updating the alt tags. Looks good to me.

[oluwatobiss]

LGTM. Thanks, @briangesteban.