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.

Sign up for GitHub

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

Jump to bottom

fix(css): Update rgb hsl values as per the latest syntax #158

Merged
merged 4 commits into from
Nov 28, 2023
Merged

fix(css): Update rgb hsl values as per the latest syntax #158

merged 4 commits into from
Nov 28, 2023

Conversation

dipikabh
Copy link
Contributor

@dipikabh dipikabh commented Nov 24, 2023 •
edited
Loading

Updates the rgb() and a few hsl() values to follow the latest syntax:

  • rgba -> rgb
  • Commas are omitted.
  • The alpha value is specified as a percentage separated by a slash (/).

UPDATE: PR to update prose/examples in content --> mdn/content#30604
@dipikabh dipikabh requested a review from bsmth November 24, 2023 01:07
bsmth
bsmth approved these changes Nov 27, 2023
View reviewed changes
Copy link
Member

@bsmth bsmth left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great, tnx. We might want to check in the content repo if we refer to any of these examples in this way:

in the following example, we use rgb(1,2,3) or rgba(1,2,3,4)

edit: indeed we need to update some places in content that reference the examples in prose.

Need update:

  • Learn/CSS/CSS_layout/Positioning
  • Learn/CSS/Building_blocks/Backgrounds_and_borders
  • Learn/CSS/Building_blocks/Advanced_styling_effects
  • Learn/CSS/CSS_layout/Media_queries
    • (Walkthrough: a simple mobile-first layout)

Look okay to me:

  • Learn/CSS/Building_blocks/The_box_model
  • Learn/CSS/Building_blocks/Debugging_CSS
  • Learn/CSS/CSS_layout/Floats_skills
  • Learn/CSS/CSS_layout/Grid_skills
@teoli2003
Copy link
Member

I think that they are missing returns at the end of some of these files.
teoli2003
teoli2003 reviewed Nov 27, 2023
View reviewed changes
learn/backgrounds-borders/background.html Outdated Show resolved Hide resolved
learn/backgrounds-borders/borders.html Outdated Show resolved Hide resolved
learn/backgrounds-borders/color.html Outdated Show resolved Hide resolved
learn/backgrounds-borders/gradients.html Outdated Show resolved Hide resolved
learn/box-model/border.html Outdated Show resolved Hide resolved
learn/box-model/inline-block-nav.html Outdated Show resolved Hide resolved
learn/images/filter-text.html Outdated Show resolved Hide resolved
@bsmth
Copy link
Member

bsmth commented Nov 27, 2023

I think that they are missing returns at the end of some of these files.

yeah let's add in this PR. I will commit your suggestions, tnx
@bsmth @teoli2003
Apply suggestions from code review
568f0f1 
Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>
@bsmth
Copy link
Member

bsmth commented Nov 27, 2023

I think that they are missing returns at the end of some of these files.

I've done the rest in bfe1480
@bsmth bsmth added the needs content update Needs update to the content to support this change. label Nov 27, 2023
@dipikabh
Copy link
Contributor Author

Thanks for reviewing, @bsmth.

edit: indeed we need to update some places in content that reference the examples in prose.

Yeah, let me open a content PR alongside before merging this one
@dipikabh dipikabh mentioned this pull request Nov 28, 2023
Merged
@dipikabh
Copy link
Contributor Author

dipikabh commented Nov 28, 2023 •
edited
Loading

Updated the following:

  • Learn/CSS/Building_blocks/Backgrounds_and_borders
  • Learn/CSS/Building_blocks/Advanced_styling_effects
  • Learn/CSS/Building_blocks/Values_and_units
  • Learn/CSS/CSS_layout/Media_queries
  • Learn/CSS/CSS_layout/Positioning

mdn/content#30604
@dipikabh
Copy link
Contributor Author

Merging this one now
@dipikabh dipikabh merged commit 3c6bfc2 into mdn:main Nov 28, 2023
@dipikabh dipikabh mentioned this pull request Dec 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs content update Needs update to the content to support this change.
3 participants
@dipikabh @teoli2003 @bsmth