-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
UPDATE: Readme file update with specificity,Responsiveness and common pitfalls #3053
Draft
Sandhya-Madhuri
wants to merge
1
commit into
airbnb:master
Choose a base branch
from
Sandhya-Madhuri:update-readme
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,6 +9,9 @@ | |||||
1. [Nesting](#nesting) | ||||||
1. [Inline](#inline) | ||||||
1. [Themes](#themes) | ||||||
1. [Specificity](#specificity) | ||||||
1. [Responsiveness](#responsiveness) | ||||||
1. [Common Pitfalls](#common-pitfalls) | ||||||
|
||||||
## Naming | ||||||
|
||||||
|
@@ -430,3 +433,185 @@ | |||||
--- | ||||||
|
||||||
CSS puns adapted from [Saijo George](https://saijogeorge.com/css-puns/). | ||||||
|
||||||
## Specificity | ||||||
|
||||||
- Handle CSS specificity carefully to avoid conflicts. | ||||||
|
||||||
> Why? Specificity can cause styles to be overridden unintentionally. In CSS-in-JavaScript, the order in which style objects are applied matters, and higher specificity selectors can lead to unexpected results. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = { | ||||||
button: { | ||||||
color: "red", | ||||||
}, | ||||||
".specialButton": { | ||||||
color: "blue", | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = { | ||||||
button: { | ||||||
color: "red", | ||||||
}, | ||||||
specialButton: { | ||||||
color: "blue", | ||||||
}, | ||||||
}; | ||||||
``` | ||||||
|
||||||
- Avoid using !important. | ||||||
|
||||||
> Why? It’s generally better to manage specificity through more precise selectors rather than forcing the style with !important. Using !important reduces maintainability by making it harder to override the styles later on. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = { | ||||||
button: { | ||||||
color: "blue !important", | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = { | ||||||
button: { | ||||||
color: "blue", | ||||||
}, | ||||||
}; | ||||||
``` | ||||||
|
||||||
--- | ||||||
|
||||||
## Responsiveness | ||||||
|
||||||
- Use media queries effectively by keeping them consistent across components. | ||||||
|
||||||
> Why? Inconsistent usage of media queries can lead to a disjointed user experience. Keeping media queries defined in a centralized location (like a theme) helps maintain uniformity. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = { | ||||||
container: { | ||||||
width: "100%", | ||||||
"@media (max-width: 600px)": { | ||||||
width: "50%", | ||||||
}, | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = (theme) => ({ | ||||||
container: { | ||||||
width: "100%", | ||||||
[theme.breakpoints.small]: { | ||||||
width: "50%", | ||||||
}, | ||||||
}, | ||||||
}); | ||||||
``` | ||||||
|
||||||
- Build responsive components using percentage-based widths and dynamic units. | ||||||
|
||||||
> Why? Responsive designs should scale smoothly across devices. Using flexible units like percentages or viewport-based units ensures components can adapt to various screen sizes. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = { | ||||||
container: { | ||||||
width: "600px", | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = { | ||||||
container: { | ||||||
width: "100%", | ||||||
maxWidth: "90vw", | ||||||
}, | ||||||
}; | ||||||
``` | ||||||
|
||||||
--- | ||||||
|
||||||
## Common Pitfalls | ||||||
|
||||||
- Overlapping styles: Be careful with conflicting styles in different parts of the component tree. | ||||||
|
||||||
> Why? Overlapping styles can result in conflicts that are hard to track down, especially in large applications. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = { | ||||||
button: { | ||||||
color: "red", | ||||||
}, | ||||||
container: { | ||||||
button: { | ||||||
color: "blue", | ||||||
}, | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = { | ||||||
button: { | ||||||
color: "red", | ||||||
}, | ||||||
specialButton: { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
color: "blue", | ||||||
}, | ||||||
}; | ||||||
``` | ||||||
|
||||||
- Style objects with conditional logic: Avoid placing too much logic into style objects. | ||||||
|
||||||
> Why? While it's possible to use JavaScript conditions to modify styles dynamically, overusing them can lead to cluttered, hard-to-read code. | ||||||
|
||||||
```js | ||||||
// bad | ||||||
const styles = (isActive) => ({ | ||||||
button: { | ||||||
color: isActive ? "green" : "red", | ||||||
}, | ||||||
}); | ||||||
|
||||||
// good | ||||||
const styles = { | ||||||
buttonActive: { | ||||||
color: "green", | ||||||
}, | ||||||
buttonInactive: { | ||||||
Comment on lines
+582
to
+585
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, should these be class names? |
||||||
color: "red", | ||||||
}, | ||||||
}; | ||||||
``` | ||||||
|
||||||
- Forgetting to centralize breakpoints: Always define breakpoints and styles centrally. | ||||||
|
||||||
> Why? Managing breakpoints centrally helps in maintaining consistent responsiveness across your app and avoids duplicate logic. | ||||||
|
||||||
```js | ||||||
Copy code | ||||||
// bad | ||||||
const styles = { | ||||||
button: { | ||||||
width: '100%', | ||||||
'@media (max-width: 500px)': { | ||||||
width: '50%', | ||||||
}, | ||||||
}, | ||||||
}; | ||||||
|
||||||
// good | ||||||
const styles = (theme) => ({ | ||||||
button: { | ||||||
width: '100%', | ||||||
[theme.breakpoints.small]: { | ||||||
width: '50%', | ||||||
}, | ||||||
}, | ||||||
}); | ||||||
|
||||||
``` |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this an improvement?
.specialButton
targets a class, andspecialButton
targets a nonexistent tag name. did you mean'button.specialButton'
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specialButton in the second is just a property to styles. It is not a class we pass it using styles.specialButton. It is a css-in-js syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you elaborate? I’m not familiar with any css-in-js solution that conflates tag names and class names in this way.