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

Add <b>, <i>, <u>, <em>, and <strong> elements #1368

Merged
merged 13 commits into from
Jul 26, 2024
Merged

Add <b>, <i>, <u>, <em>, and <strong> elements #1368

merged 13 commits into from
Jul 26, 2024

Conversation

petele
Copy link
Collaborator

@petele petele commented Jul 11, 2024

No description provided.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Jul 11, 2024
foolip
foolip reviewed Jul 12, 2024
View reviewed changes
Copy link
Collaborator

@foolip foolip left a comment

Choose a reason for hiding this comment

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

I wonder if we should throw these text formatting elements into a single feature or not? And what about <s>? I can't make up my mind. @ddbeck?

features/b.yml Outdated Show resolved Hide resolved
captainbrosset
captainbrosset previously requested changes Jul 15, 2024
View reviewed changes
features/b.yml Outdated Show resolved Hide resolved
ddbeck
ddbeck reviewed Jul 15, 2024
View reviewed changes
features/b.yml Outdated Show resolved Hide resolved
features/b.yml Outdated Show resolved Hide resolved
features/i.yml Outdated Show resolved Hide resolved
@petele
Copy link
Collaborator Author

petele commented Jul 15, 2024

I split these each into their individual elements, and updated the descriptions based on the feedback provided.

@foolip
Copy link
Collaborator

foolip commented Jul 22, 2024

@petele there are conflicts due to #1482. Can you rebase and update dist?

foolip
foolip reviewed Jul 23, 2024
View reviewed changes
features/b.yml.dist Outdated Show resolved Hide resolved
foolip
foolip approved these changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@foolip foolip left a comment

Choose a reason for hiding this comment

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

@ddbeck do you want to review the descriptions too? I think we'll be setting some precedents here.

features/em.yml Outdated Show resolved Hide resolved
features/u.yml Outdated Show resolved Hide resolved
features/strong.yml Outdated Show resolved Hide resolved
features/i.yml Outdated Show resolved Hide resolved
features/b.yml Outdated Show resolved Hide resolved
@foolip foolip mentioned this pull request Jul 23, 2024
Merged
@petele petele requested a review from foolip July 23, 2024 14:13
ddbeck
ddbeck reviewed Jul 23, 2024
View reviewed changes
features/em.yml Outdated Show resolved Hide resolved
features/strong.yml Outdated Show resolved Hide resolved
features/b.yml Outdated Show resolved Hide resolved
features/i.yml Outdated Show resolved Hide resolved
features/u.yml Outdated Show resolved Hide resolved
@petele petele requested a review from ddbeck July 23, 2024 15:39
foolip
foolip approved these changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Let's do it! Styling text by default!

@foolip
Copy link
Collaborator

foolip commented Jul 23, 2024

@captainbrosset do you want to review this again? You have previously requested changes, and I'd need to dismiss that review in order to merge.

@petele petele changed the title Add b, i, and u elements Add <b>, <i>, <u>, <em>, and <strong> elements Jul 25, 2024
@foolip foolip dismissed captainbrosset’s stale review July 25, 2024 16:35

Further discussion led to individual features and descriptions. Grouping them later could make sense.

@petele petele merged commit 4c5d7b0 into web-platform-dx:main Jul 26, 2024
3 checks passed
@petele petele deleted the elem-b-i-u branch July 26, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddbeck ddbeck ddbeck approved these changes

@foolip foolip foolip approved these changes

@captainbrosset captainbrosset Awaiting requested review from captainbrosset

Assignees
No one assigned
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@petele @foolip @ddbeck @captainbrosset