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(html): update positioning and scaling section #34529

Merged
merged 8 commits into from
Jul 15, 2024
Merged

fix(html): update positioning and scaling section #34529

merged 8 commits into from
Jul 15, 2024

Conversation

OnkarRuikar
Copy link
Contributor

The object-fit doesn't work at all in any browser. And object-position works only in Firefox.

In the OWD meeting, it was decided to remove object-fit from the section. And update BCD of object-position to show that, for iframe, it's supported only in Firefox.
@OnkarRuikar OnkarRuikar requested a review from a team as a code owner July 1, 2024 07:01
@OnkarRuikar OnkarRuikar requested review from dipikabh and removed request for a team July 1, 2024 07:01
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs size/s [PR only] 6-50 LoC changed labels Jul 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2024 •
edited
Loading

Preview URLs

(comment last updated: 2024-07-15 01:10:15)
@Josh-Cena
Copy link
Member

Josh-Cena commented Jul 1, 2024 •
edited
Loading

This is also mentioned in <embed> and <fencedframe> (I would just assume other image-related elements do support these two properties). Should we update those too (given tests)?

I don't think we should just take it out; in the meeting we also mentioned adding an explicit note saying it doesn't work, which IMO is better.

Also the object-fit says it works for all replaced elements which then doesn't seem true.

w3c/csswg-drafts#7143 is relevant here.
@dipikabh dipikabh self-assigned this Jul 10, 2024
dipikabh
dipikabh requested changes Jul 10, 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.

@OnkarRuikar, thanks for working on this reported issue!

Please see my suggestions for the iframe page. If you agree, they can be applied to the other two pages as well.

I think we should add a similar note on the object-fit page as well since it does not apply to all replaced elements.
files/en-us/web/html/element/iframe/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/iframe/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/iframe/index.md Outdated Show resolved Hide resolved
OnkarRuikar and others added 2 commits July 11, 2024 09:47
@OnkarRuikar @dipikabh
Apply suggestions from code review
17fd298 
Co-authored-by: Dipika Bhattacharya <dipika@foss-community.org>
@OnkarRuikar
add note on object-fit page
62ff634
@OnkarRuikar OnkarRuikar requested a review from a team as a code owner July 11, 2024 04:36
@OnkarRuikar OnkarRuikar requested review from bsmth and removed request for a team July 11, 2024 04:36
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Jul 11, 2024
@bsmth
Copy link
Member

bsmth commented Jul 11, 2024

Removing myself from review because Dipika and Josh are on it!
@bsmth bsmth removed their request for review July 11, 2024 07:30
dipikabh
dipikabh requested changes Jul 11, 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.

Thanks @OnkarRuikar. It's close, just a matter of replicating updates to <embed> and <fencedframe>.
files/en-us/web/css/object-fit/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/embed/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/fencedframe/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/fencedframe/index.md Outdated Show resolved Hide resolved
@OnkarRuikar
Copy link
Contributor Author

It's close, just a matter of replicating updates to <embed> and <fencedframe>.

My bad. I had this in mind but it slipped my mind in a split second.

I think we can leave out <fencedframe> because it is experimental

Lets future future-proof the content by keeping it. If the fencedframe changes the behavior, we'll update accordingly.

Not being in the replaced elements list is a bug that should be fixed separately

We have to update the page here itself as it also contains a section about object-fit.
dipikabh
dipikabh requested changes Jul 15, 2024
View reviewed changes
files/en-us/web/css/object-fit/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/replaced_element/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/embed/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/fencedframe/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/iframe/index.md Outdated Show resolved Hide resolved
dipikabh
dipikabh approved these changes Jul 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.

Thanks, @OnkarRuikar!
@dipikabh dipikabh merged commit 71f57bc into mdn:main Jul 15, 2024
8 checks passed
@OnkarRuikar OnkarRuikar deleted the html_iframe_object_fit branch July 15, 2024 04:42
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 Content:HTML Hypertext Markup Language docs size/s [PR only] 6-50 LoC changed
4 participants
@OnkarRuikar @Josh-Cena @bsmth @dipikabh