Updated/Removed todos in Codebase Best Practices page #552
Conversation
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
- Removed todo comment about practice that is already implemented. - Fixed mistakes - Removed todo comment about store-related practice as it is not applicable.
Comment on lines
-1
to
-5
| --- | ||
| title: Codebase Best Practices | ||
| --- | ||
| # Codebase Best Practices | ||
|
|
||
| import { Steps } from '@astrojs/starlight/components'; |
There was a problem hiding this comment.
isn't this needed for the astro build?
|
|
||
| ## Styling a component | ||
|
|
||
| We recommend styling components using our [design style guide](https://design-style-guide.freecodecamp.org/). | ||
| We recommend styling components using our [`design style guide`](https://design-style-guide.freecodecamp.org/). |
There was a problem hiding this comment.
Suggested change
| We recommend styling components using our [`design style guide`](https://design-style-guide.freecodecamp.org/). | |
| We recommend styling components using our [design style guide](https://design-style-guide.freecodecamp.org/). |
| @@ -20,13 +17,13 @@ We are striving to support right-to-left (RTL) layout in the codebase for langua | |||
|
|
|||
| - Don't use `float` properties | |||
| - Use Flexbox and Grid layouts instead, as they have RTL support already built-in, and those will be easier to maintain and review. | |||
| - Don't define the direction while using `margin` and `padding`: it may seem harmless to use `padding-right` and `margin-left`, but these directions aren't mirrored when the layout changes to RTL, and adding counter values for them in the RTL file makes maintaining the codebase harder. | |||
| - Don't define the direction while using `margin` and `padding` it may seem harmless to use `padding-right` and `margin-left`, but these directions aren't mirrored when the layout changes to RTL, and adding counter values for them in the RTL file makes maintaining the codebase harder. | |||
There was a problem hiding this comment.
Suggested change
| - Don't define the direction while using `margin` and `padding` it may seem harmless to use `padding-right` and `margin-left`, but these directions aren't mirrored when the layout changes to RTL, and adding counter values for them in the RTL file makes maintaining the codebase harder. | |
| - Don't define the direction while using `margin` and `padding`: it may seem harmless to use `padding-right` and `margin-left`, but these directions aren't mirrored when the layout changes to RTL, and adding counter values for them in the RTL file makes maintaining the codebase harder. |
why did you remove the colon?
| - Use logical properties for them: You can add the same spacing by using `padding-inline-end` and `margin-inline-start`, and you won't need to worry about RTL layout, as they follow where the line starts and ends, and you won't need to add any extra values in the RTL files, so people won't need to remember to change the same values in two files. | ||
| - Don't use `!important` in `font-family`: RTL layout uses different fonts compared to the LTR layout, when you add `!important` in the `font-family` property it affects the RTL layout too. | ||
| - Don't use `!important` in `font-family` RTL layout uses different fonts compared to the LTR layout, when you add `!important` in the `font-family` property it affects the RTL layout too. |
There was a problem hiding this comment.
Suggested change
| - Don't use `!important` in `font-family` RTL layout uses different fonts compared to the LTR layout, when you add `!important` in the `font-family` property it affects the RTL layout too. | |
| - Don't use `!important` in `font-family`: RTL layout uses different fonts compared to the LTR layout, when you add `!important` in the `font-family` property it affects the RTL layout too. |
same here, why?
|
|
||
| ## General JavaScript | ||
|
|
||
| In most cases, our [linter](/how-to-setup-freecodecamp-locally/#follow-these-steps-to-get-your-development-environment-ready) will warn of any formatting which goes against this codebase's preferred practice. | ||
| In most cases, our [`linter`](/how-to-setup-freecodecamp-locally/#follow-these-steps-to-get-your-development-environment-ready) will warn of any formatting which goes against this codebase's preferred practice. |
There was a problem hiding this comment.
Suggested change
| In most cases, our [`linter`](/how-to-setup-freecodecamp-locally/#follow-these-steps-to-get-your-development-environment-ready) will warn of any formatting which goes against this codebase's preferred practice. | |
| In most cases, our [linter](/how-to-setup-freecodecamp-locally/#follow-these-steps-to-get-your-development-environment-ready) will warn of any formatting which goes against this codebase's preferred practice. |
Comment on lines
-52
to
-56
| :::note | ||
| note | ||
| Editors like VSCode are still likely to show you the file has been deleted | ||
| and a new one created. If you use the CLI to `git add .`, then VSCode will show the | ||
| file as renamed in the staging area. | ||
| ::: |
There was a problem hiding this comment.
isn't this the way astro make the note blocks? why did you remove it?
| @@ -135,7 +129,7 @@ export const reducer = ( | |||
|
|
|||
| ### How to Dispatch | |||
|
|
|||
| Within a component, import the actions and selectors needed. | |||
| With in a component, import the actions and selectors needed. | |||
There was a problem hiding this comment.
Suggested change
| With in a component, import the actions and selectors needed. | |
| Within a component, import the actions and selectors needed. |
Within is perfectly good English
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Checklist:
Update index.md)Closes #393