Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Styling React Applications: Update styling recommendations #27874

Merged
merged 8 commits into from May 10, 2024

Conversation

Siriuszx
Copy link
Contributor

@Siriuszx Siriuszx commented Apr 27, 2024

Because

The lesson tells people that Tailwind CSS is a valid Vanilla CSS option which isn't completely untrue, however the general consensus is that students should refrain from utilizing CSS frameworks or component libraries and try writing their own css.

This PR

  • reworded lesson tip
  • removed Tailwind CSS docs link from Additional Resources
  • added navigation links that point to the list of styling options available for students to choose from

Issue

Closes #27867

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: React Involves the React course label Apr 27, 2024
Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

Thanks for doing this PR. Would you mind fixing up the lint issues as well? They should be pretty self explanatory. Let me know if you run into any issues with this.

@@ -30,7 +30,7 @@ What if everything's already done for you? Styling, behavior, and accessibility

<div class="lesson-note lesson-note--tip" markdown="1" >

For learning purposes throughout this course, we advise you to implement your component's styling from scratch i.e. use vanilla CSS or a CSS-in-JS option.
For the learning purposes throughout this course, we recommend that you avoid using CSS frameworks or component libraries, and instead implement your component's styling from scratch i.e. use [CSS Modules](#vanilla-css) or a [CSS-in-JS](#css-in-js) option.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For the learning purposes throughout this course, we recommend that you avoid using CSS frameworks or component libraries, and instead implement your component's styling from scratch i.e. use [CSS Modules](#vanilla-css) or a [CSS-in-JS](#css-in-js) option.
For learning purposes throughout this course, we recommend that you avoid using CSS frameworks or component libraries, and instead implement your component's styling from scratch i.e. use [CSS Modules](#vanilla-css) or a [CSS-in-JS](#css-in-js) option.

Nit: "For learning purposes" (omitting "the") is more appropriate here.

Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

In all honesty, the more I think about it, the more unsure I am about having Tailwind within Vanilla CSS, because it isn't vanilla CSS by any means. CSS modules can be argued as "for all intents and purposes, vanilla CSS" since it's almost the same, just with very tiny differences like local scoping.

I feel Tailwind ought to go in its own ### Utility frameworks between ### CSS in JS and ### Component libraries.

  • The Vanilla CSS section would need a very slight rewording as it's no long highlighting "a couple of things", and it won't need an ordered list anymore. CSS modules can just be a normal sentence/paragraph in it.
  • The Tailwind list item can be made its own paragraph in its own ### Utility frameworks section.

What do you think, @Siriuszx?

@Siriuszx
Copy link
Contributor Author

Siriuszx commented Apr 28, 2024

I think the definition of "Vanilla CSS" is too vague and purely subjective in that context.

To avoid confusion in the future, I think it would be better if we just get rid of the term "Vanilla CSS" altogether and split the lesson into:

  • CSS Modules
  • CSS Utility Frameworks
  • CSS-in-JS
  • Component Libraries

I understand what the author of this lesson was thinking when listing tailwind as vanilla css, but categorizing ALL of the frameworks as vanilla css is just wrong I think.

@Siriuszx
Copy link
Contributor Author

Siriuszx commented Apr 28, 2024

If we are on the same page about this could you please merge this PR into main and I will open a separate PR addressing this lesson's structure? I would be grateful if I get the chance to work on the next problem as well 🙂

@MaoShizhong
Copy link
Contributor

@Siriuszx I think this is pretty directly related to the original issue and PR purpose, so I think it can be done in this PR instead of a separate one.

I like your idea about the sections so let's go with that.
I think the Introduction section could do with a slight update to include something about vanilla CSS potentially getting harder to manage in a larger project due to every file sharing a global scope. Therefore there are some popular alternatives in common use.

I think that'll help set the scene for why something like css modules is so useful because of the local scoping, while being otherwise almost vanilla css.

I'll leave that with you, and we can always refine in review if we need to.

@MaoShizhong MaoShizhong self-assigned this Apr 30, 2024
@Siriuszx
Copy link
Contributor Author

Siriuszx commented May 4, 2024

@MaoShizhong sorry if this is taking too long, it says that you've "self-assigned this". I was a little bit busy with life, should I continue working on this issue?

@MaoShizhong
Copy link
Contributor

Oh yeah, carry on with it @Siriuszx. The PR assignment is a maintainer thing, you can ignore it

Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

Love this! Just a few nits/suggestions below.


### Vanilla CSS
### CSS Modules
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### CSS Modules
### CSS modules

Just need to use sentence case

@@ -1,36 +1,36 @@
### Introduction

Up until now you've been using vanilla CSS to style your React projects, but there are other options available. This lesson is intended to be more like a guide or a list of options for you to explore.
In the previous courses, you've learned a lot of CSS and all of those skills are still applicable to React, however there are a couple of things we'd like to highlight. As you've probably already noticed, all of the styles we write share the global scope, which means that as our application grows, it becomes increasingly difficult to manage our CSS. Some of the tools mentioned below will help solve this problem, however it's important to note that the main purpose of this lesson is to be more of a guide or a list of available options for you to explore when it comes to styling React applications.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In the previous courses, you've learned a lot of CSS and all of those skills are still applicable to React, however there are a couple of things we'd like to highlight. As you've probably already noticed, all of the styles we write share the global scope, which means that as our application grows, it becomes increasingly difficult to manage our CSS. Some of the tools mentioned below will help solve this problem, however it's important to note that the main purpose of this lesson is to be more of a guide or a list of available options for you to explore when it comes to styling React applications.
In the previous courses, you'll have learned a lot of CSS and all of those skills are still applicable to React. However, there are a couple of things we'd like to highlight. As you've probably already noticed, all of the styles we write share a global scope, which means that as our application grows, it will become increasingly difficult to manage our CSS. Some of the tools mentioned below are things people use to help solve this problem.

Just a few grammar/flow nits, and I feel the last sentence isn't really necessary to say. Just a small tweak to the sentence before it should let it end very nicely there, I feel.


### CSS in JS
CSS-in-JS is a paradigm for styling front-end projects. It allows you to entirely take control of CSS with JavaScript and extends it with various features. Additionally, it also helps to apply styling in a logical fashion i.e. based on state, and also supports modular CSS in the same way that CSS Modules do. There are various CSS-in-JS solutions. One of the most popular ones in the React ecosystem is [styled-components](https://styled-components.com/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CSS-in-JS is a paradigm for styling front-end projects. It allows you to entirely take control of CSS with JavaScript and extends it with various features. Additionally, it also helps to apply styling in a logical fashion i.e. based on state, and also supports modular CSS in the same way that CSS Modules do. There are various CSS-in-JS solutions. One of the most popular ones in the React ecosystem is [styled-components](https://styled-components.com/).
CSS-in-JS is a paradigm for styling front-end projects. It allows you to entirely take control of CSS with JavaScript and extends it with various features. Additionally, it also helps to apply styling in a logical fashion, e.g. based on state, and also supports modular CSS in the same way that CSS Modules do. There are various CSS-in-JS solutions. One of the most popular ones in the React ecosystem is [styled-components](https://styled-components.com/).

Nit


Vanilla CSS is the simplest way to style. In the previous courses, you've learned a lot of CSS and all of those skills are applicable to React. There are a couple of things we'd like to highlight.
Regular CSS is the simplest way to style. CSS Modules let you write CSS style declarations that are scoped locally, which means that we finally no longer have to worry about our class names potentially conflicting with other classes in the global scope, i.e. we can now have two (or more) classes with the same name but different styles, and avoid naming your classes `.list-wrapper`, `.item-wrapper` etc. and just use `.wrapper` with its respective component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Regular CSS is the simplest way to style. CSS Modules let you write CSS style declarations that are scoped locally, which means that we finally no longer have to worry about our class names potentially conflicting with other classes in the global scope, i.e. we can now have two (or more) classes with the same name but different styles, and avoid naming your classes `.list-wrapper`, `.item-wrapper` etc. and just use `.wrapper` with its respective component.
Regular CSS is the simplest way to style. CSS Modules let you write CSS style declarations that are scoped locally, which means that we finally no longer have to worry about our class names potentially conflicting with other classes in the global scope. For example, we can have several CSS module files containing a `.wrapper` class, but each module's `.wrapper` will be treated as different to each other.

The above is purely a suggestion, so you don't have to take it at all. Just mainly feeling like it'd be best to just briefly explain how a class in one CSS module is different to the same class name in another CSS module.
I'm not sure the current example is 100% applicable (I'm being very nitpicky here!), since you may still want to use class names like list-wrapper and item-wrapper between/within CSS modules for various reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I guess it could be even redundant. People will find explanations for what it really is when they eventually get to the assignment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, especially when the documentation for CSS modules is already linked as an assigned reading.
We could probably just leave it at something like

Regular CSS is the simplest way to style. CSS Modules let you write locally scoped CSS style declarations, preventing many issues with clashing class names in the global scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already done! :)

Agreed, especially when the documentation for CSS modules is already linked as an assigned reading. We could probably just leave it at something like

Regular CSS is the simplest way to style. CSS Modules let you write locally scoped CSS style declarations, preventing many issues with clashing class names in the global scope.

Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

🚀

@MaoShizhong MaoShizhong merged commit 933a127 into TheOdinProject:main May 10, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: React Involves the React course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Styling React Applications(React): clarifications regarding Tailwind CSS as a styling option
2 participants