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

36125 add text emphasis position auto #36331

Merged
merged 33 commits into from
Oct 29, 2024
Merged

36125 add text emphasis position auto #36331

merged 33 commits into from
Oct 29, 2024

Conversation

dletorey
Copy link
Contributor

@dletorey dletorey commented Oct 14, 2024 •
edited
Loading

Description

  • Added auto value to Values
  • Added auto into Syntax
  • Added explanation of ruby
  • Added an extra example

Motivation

Working on issue #36125

Additional details

Related issues and pull requests

@dletorey dletorey requested a review from a team as a code owner October 14, 2024 15:51
@dletorey dletorey requested review from estelle and dipikabh and removed request for a team and estelle October 14, 2024 15:51
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Oct 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 14, 2024 •
edited
Loading

Preview URLs

(comment last updated: 2024-10-29 16:00:22)

@dletorey dletorey requested a review from bsmth October 14, 2024 15:56
This was referenced Oct 14, 2024
Merged
Closed
@jfkthame
Copy link

jfkthame commented Oct 14, 2024 •
edited
Loading

Preview URLs

In this preview, it looks like the "Formal definition" and "Formal syntax" sections still need an update.

@dletorey
Copy link
Contributor Author

dletorey commented Oct 14, 2024 •
edited
Loading

In this preview, it looks like the "Formal definition" and "Formal syntax" sections still need an update.

Those can be found in the following Pull Request:

dipikabh
dipikabh requested changes Oct 15, 2024
View reviewed changes
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.

Thank you for making these updates, @dletorey!

I've added some suggestions and questions

files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Show resolved Hide resolved
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Oct 16, 2024
dletorey and others added 6 commits October 16, 2024 09:59
dletorey and others added 3 commits October 16, 2024 11:56
@dletorey @dipikabh
Update files/en-us/web/css/text-emphasis-position/index.md
dfd9587 
Co-authored-by: Dipika Bhattacharya <[email protected]>
@dletorey @github-actions
Update files/en-us/web/css/text-emphasis-position/index.md
f293d17 
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@dletorey @github-actions
Update files/en-us/web/css/text-emphasis-position/index.md
a39c559 
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@dletorey dletorey mentioned this pull request Oct 16, 2024
Merged
dipikabh
dipikabh approved these changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@dipikabh dipikabh 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.

Thanks for the updates, @dletorey. They're looking good so leaving a +1.

A few nits need to be resolved before this PR is merged. I'm not seeing the option to apply the suggested changes else I would have committed them.

Update: The writing-mode property can be added to the "See also". And maybe also text-underline-position.

@dletorey dletorey requested a review from dipikabh October 17, 2024 10:12
dipikabh
dipikabh requested changes Oct 18, 2024
View reviewed changes
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.

Sorry, @dletorey, there's still a few things that need to be fixed (missing H2 and missing bullets in "Values" section). The suggestions in "See also" are optional but can applied to improve that section while we're here. Thanks!

files/en-us/web/css/text-emphasis-position/index.md Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-emphasis-position/index.md Outdated Show resolved Hide resolved
@dletorey dletorey requested a review from dipikabh October 28, 2024 15:09
dipikabh
dipikabh requested changes Oct 28, 2024
View reviewed changes
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.

I've left a suggestion for the Values section, @dletorey.
Thank you for the other fixes, they look good.

@dletorey
Copy link
Contributor Author

I'll sort it first thing in the morning

@dletorey dletorey requested a review from dipikabh October 29, 2024 15:59
dipikabh
dipikabh approved these changes Oct 29, 2024
View reviewed changes
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.

Perfect, thanks a lot @dletorey 🎉

@dipikabh dipikabh merged commit 8750e12 into mdn:main Oct 29, 2024
8 checks passed
@dletorey dletorey deleted the 36125-add-text-emphasis-position-auto branch November 12, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@dipikabh dipikabh dipikabh approved these changes

@bsmth bsmth Awaiting requested review from bsmth

Assignees
No one assigned
Labels
Content:CSS Cascading Style Sheets docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dletorey @jfkthame @dipikabh