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

fix(css): update 'drop-shadow()' page #29013

Merged
merged 6 commits into from
Sep 25, 2023
Merged

Conversation

OnkarRuikar
Copy link
Contributor

It is helpful to document the required parameters first then the optional parameters. For usage in code no guideline exist about the order. Also Prettier has no opinion about this. We can show all allowed combinations of values in the "Syntax" section.

The PR does following changes:

  • updates the syntax section to cover various scenarios
  • uses {{optional_inline}} badge. All non optional params are mandatory.
  • adds playable example
  • use standard-deviation instead of blur-radius. The drop-shadow and box-shadow implementations use different algorithms.

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner September 9, 2023 06:01
@OnkarRuikar OnkarRuikar requested review from dipikabh and removed request for a team September 9, 2023 06:01
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Sep 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2023

Preview URLs

External URLs (1)

URL: /en-US/docs/Web/CSS/filter-function/drop-shadow
Title: drop-shadow()

(comment last updated: 2023-09-22 19:08:17)

@dipikabh dipikabh self-assigned this Sep 12, 2023
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thanks @OnkarRuikar for adding these improvements to the page!

files/en-us/web/css/filter-function/drop-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/filter-function/drop-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/filter-function/drop-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/filter-function/drop-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box-shadow/index.md Show resolved Hide resolved
files/en-us/web/css/box-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box-shadow/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thanks, @OnkarRuikar. It is getting pretty close. Just a few more adjustments needed for the box-shadow page.

files/en-us/web/css/box-shadow/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box-shadow/index.md Outdated Show resolved Hide resolved
- `<color>`
- : See {{cssxref("&lt;color&gt;")}} values for possible keywords and notations.
If not specified, it defaults to {{cssxref("&lt;color&gt;","currentcolor","#currentcolor_keyword")}}.
- The fourth optional {{cssxref("&lt;length&gt;")}} value is interpreted as `<spread-radius>`. Positive values will cause the shadow to expand and grow bigger, negative values will cause the shadow to shrink. If not specified, it will be `0` (the shadow will be the same size as the element).
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
- The fourth optional {{cssxref("&lt;length&gt;")}} value is interpreted as `<spread-radius>`. Positive values will cause the shadow to expand and grow bigger, negative values will cause the shadow to shrink. If not specified, it will be `0` (the shadow will be the same size as the element).
- If four values are specified, the fourth value is interpreted as `<spread-radius>`. Positive values will cause the shadow to expand and grow bigger, negative values will cause the shadow to shrink. If not specified, it will be set to `0` (that is, the shadow will be the same size as the element).

Copy link
Contributor

Choose a reason for hiding this comment

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

While we're updating the value descriptions, can we also update this part just before "Values" to remove duplicate info:

Specify a single box-shadow using:

  • Two, three, or four <length> values.
    • If only two values are given, they are interpreted as <offset-x> and <offset-y> values.
    • If a third value is given, it is interpreted as a <blur-radius>.
    • If a fourth value is given, it is interpreted as a <spread-radius>.
  • Optionally, the inset keyword.
  • Optionally, a <color> value.

To specify multiple shadows, provide a comma-separated list of shadows.

  • We should probably move the above text to the "Values" section.
  • And update it to:

You can create a single box-shadow by specifying two, three, or four <length> values, an optional inset, and an optional <color> value. To create multiple box-shadows, specify a comma-separated list of shadows for the box-shadow property.

files/en-us/web/css/box-shadow/index.md Show resolved Hide resolved
Co-authored-by: Dipika Bhattacharya <[email protected]>
Copy link
Contributor

@dipikabh dipikabh 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 your work on these update, @OnkarRuikar 🙌

@dipikabh dipikabh merged commit 4599021 into mdn:main Sep 25, 2023
6 checks passed
@OnkarRuikar OnkarRuikar deleted the css_filter branch September 26, 2023 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drop-shadow() parameters
3 participants