Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

contect rect definition is strange #54

Closed
dbaron opened this issue Feb 2, 2018 · 19 comments
Closed

contect rect definition is strange #54

dbaron opened this issue Feb 2, 2018 · 19 comments

Comments

@dbaron
Copy link

dbaron commented Feb 2, 2018

The definition of content rect seems pretty strange. It seems (although the wording is a little odd and the padding links are broken) like it's using the top and left of the CSS padding box, and the width and height of the CSS content box, which is quite strange. It seems designed to optimize for absolute positioning, but it perhaps makes things better for English and vertical Mongolian at the expense of making them worse for Arabic or vertical Chinese.

@dbaron
Copy link
Author

dbaron commented Feb 2, 2018

(I got here from w3ctag/design-reviews#187.)

@dbaron
Copy link
Author

dbaron commented Feb 2, 2018

I'd also add that it should be clear what "padding top" and "padding left" are relative to -- is it the element or its parent? (If it's the element, then is it really worth caring about when top and left change at all? That would be detecting only changes to the border.)

It seems likely to be intended as the element given the definition for SVGGraphicsElement, which says top and left are always 0.

@gregwhitworth
Copy link

I agree here, I recently did a talk that included a quick section on ResizeObserver and this being the first time I had actually used this I assumed it WAS the content rect, not the contentRect & the paddingRect blended together. Since many authors are aware of the differences between these due to dev tools, I think we should provide the various rects with their specific info. The primary usecase for this can be broken if the dimension of the box is adjusted in any box other than the contentRect which is common (here's a basic example that I adjusted: https://codepen.io/gregwhitworth/pen/gKoMPE). I think we need all of the rects represented in some form, at least the width/height and for left/top it should be the border box only.

@eeeps
Copy link

eeeps commented Jun 18, 2018

@gregwhitworth

@atotic cited perf and simplicity/interoperability as the two reasons why Level 1 of the spec only tracks content rects, here: #34 (comment)

Is it just as true within Edge as it is within Chrome that tracking/exposing other boxes “introduces many other dependencies, that are complex to compute efficiently”?

@gregwhitworth
Copy link

@eeeps I don't see why it would be, but I haven't been involved in implementing an observer within Edge so there may be complications that I'm not privy to. Taking another lens - I think it's the right thing to do from a webdev's perspective; Chrome is already providing the padding box positioning info. I asked @FremyCompany and @atanassov to discuss this at the next CSSWG F2F in Sydney (if there is time).

@atotic
Copy link
Collaborator

atotic commented Jun 25, 2018

Apologies for the late reply, I've been off ResizeObserver for a while.

Justification for content_rect has not been succinctly explained before.

Here is my attempt to do so.

  1. Why ResizeObserver?

    • Purpose of ResizeObserver is to let webdevs manipulate Element's children in response to Element's size changes.
    • Element's children are displayed inside content rect by DOM convention.
  2. Why content rect as specified?

    • Content rect is where children are displayed, it is the size webdevs need to position children.
    • Padding left/top are content's rect location inside element's padding box. Padding box is used instead of border box, because padding box is static position rectangle for absolute position.

In response to your questions:

@dbaron

perhaps makes things better for English and vertical Mongolian at the expense of making them worse for Arabic or vertical Chinese.

You are correct, I assumed absolute positioning would use top/left to specify position. Maybe
merging content_size and padding was suboptimal. Separate content_size, and padding left/top/bottom/right would be more appropriate, and less confusing.

@gregwhitworth

The primary usecase for this can be broken if the dimension of the box is adjusted in any box other than the contentRect which is common

Could you elaborate? Is your scenario that if border_box, or padding_box change size while content_box remains the same, RO should fire a notification?

@gregwhitworth
Copy link

@atotic In working with this - I expect that webdevs will expect it to behave similar to media queries and it initially surprised me that it didn't fire. You're correct that my children may not overflow their container, but what if I want to handle both in the same place. Regarding the usecase above, in a scenario where I don't control the parent I may desire to trigger the RO to change it's border/padding styles accordingly at a specific width which MQs will not suffice. I think providing borderBox, paddingBox, and contentBox should be what is passed to the author and then let them do their logic against whatever rect they desire.

@atotic
Copy link
Collaborator

atotic commented Jun 25, 2018

@gregwhitworth I looked at your example. RO did fire, but the rectangle did not turn green because content size was < 100px. If I understand correctly, your argument is:

  • webdev would like to change border/padding in response to RO notification.
  • RO notification does not have information webdev needs (border rect/padding rect).

This can be problematic: changing self border/padding in response to RO notification changes content size, which triggers another RO notification, creating a loop. It is easy for developers to create jank this way.
Easiest way to avoid loops is to avoid modifying self, and only modify children. This is another reason for choosing content rect, it is a nudge to "Manipulate your content, not self".
I can see an argument for reporting:

  • content size
  • padding edges
  • border edges
    instead of hybrid content rect.

I have not had any direct developer feedback with real-world use case for reporting full CSS rect set.
There is a comment suggesting this, without details. I'll follow up to see if I can get any further details.

@eeeps
Copy link

eeeps commented Jun 27, 2018

@atotic @gregwhitworth

Here’s a use case for reporting at least content and border edges.

I have a proof-of-concept element/container query implementation here: https://github.com/eeeps/presto-points

It uses RO to measure the “width” of a queried box – based on that width it'll add or remove classes from an element.

I, like many (most?) web developers, am in the habit of using * { box-sizing: border-box; } on everything. When I query the width of an element, I assume that the query will think about “width” just like my rules do – i.e., width: 10em means that an element's content width + padding width + border width = 10em.

So it was very surprising to me when this .container...

* { box-sizing: border-box; }
.container {
   --presto-points: 10em .large;
   padding: 1em;
   width: 10em;
   background: bisque;
}
.container.large {
   background: tomato;
}

...was initially bisque. When I figured out that my content box was only 8em wide and that that was what RO was observing, I commented in #34 to complain, and went ahead and hacked around the RO content-box-only limitation so that things worked as I expected them to.

@atotic
Copy link
Collaborator

atotic commented Jun 28, 2018

We are in agreement that reporting border/padding/content size might cover more use cases.

I am leaning towards not watching border/padding. Padding could change without resizing of content size if padding and border box changed in sync and kept content size the same. Watching padding might be a little bit more expensive because Chrome does not explicitly store padding size, so it would have to be computed every time.

@FremyCompany
Copy link

I think the request is to have an equivalent for the properties currently defined that are based on the border-box rectangle and view section. It seems reasonable to leave out the padding issue apart, the problem here is mostly that for most devs "width" covers the border+padding+content because the usual rectangle associated with an element is getBoundingClientRect().

I would be in favor of returning properties based on top of the bounding client rect instead of (or in addition to) the content rect.

@atotic
Copy link
Collaborator

atotic commented Jun 28, 2018

Reporting border box rect might lead web developers to suboptimal usage of RO.

The intended use cases are for developer to modify contents of the observed Element.

The reason for this is if developers modify properties of the observed Element (border/padding/size),
its size might change, resulting in a loop (notification->size change -> notification->....).

@gregwhitworth
Copy link

@atotic We're aware of that potential scenario, and it is still possible today it's just that you'll have to do what @eeeps already did in order to achieve the same end result. Additionally, what we're proposing is already being utilized today it's just in utilizing with getBoundingClientRect with RAF() - so the primary thing that RO is doing is limiting the calls to only when a rect is changed and passing in the layout information without needing to fetch it, except that layout information doesn't correspond to what authors are used to. I agree with @FremyCompany that at the very least we should pass back gbcr or pass back something that is akin to what authors see in their dev tools.

@eeeps
Copy link

eeeps commented Jun 30, 2018

@atotic

I don't think that limiting notifications to content box changes helps to avoid loops in many cases. e.g.,

* { box-sizing: border box; }
div { width: 10em; }

A script that modifies the padding on the div in response to a RO notification will affect the content size as well as the padding size, here (sending you into a loop no matter which one you're watching).

The ability to modify things that you're observing inherently opens the door to loops. That's as true for content size as it is padding and border... As a low-level JS primitive, I don't think it's ROs job to prevent those loops, any more than it's JavaScript's job to prevent people from writing while( true ).

I'd expect to be able to watch for changes on the same thing that's being reported (in my case, total border+padding+content size). If ergonomics/principle-of-least-suprise reasons aren't justification enough, I'll try to think through some concrete use cases for when you'd want to watch for changes to padding/border, specifically.

@atotic
Copy link
Collaborator

atotic commented Jul 2, 2018

The ability to modify things that you're observing inherently opens the door to loops. That's as true for content size as it is padding and border... As a low-level JS primitive, I don't think it's ROs job to prevent those loops, any more than it's JavaScript's job to prevent people from writing while( true ).

The goal of RO API design was to steer web developers towards writing performant, maintainable code. Performant meant code that would not skip frames during animation. Maintainable meant that components using RO could be embedded in a page without worrying about RO loops degrading page performance.

The primary use case considered was: "a component that gets embedded in a page. Allow component to change its appearance based upon size". Ex: google-map. We imagined a page with 100s of responsive components using RO.

Loops were bad. Having components get RO notification, self-modify, and trigger another self RO notification was undesirable. Rendering of pages with unprocessed RO notifications can be janky.

Out of these constraints, we concluded that the best way to write performant, maintainable RO code was for components to never modify their own style, but only modify their children. Self-modification was too loop prone. And that is why our API reports the content rect: "Your might want to modify content based upon new size".

I understand that this API does not work well for emulating element queries. It can be done, but takes more work. This is intentional, because using the API this way can easily lead to loops.

I agree that the API could report border/padding + content size instead of our strange content rect. This would also make loops even more obvious to developers, any modification to its own border/padding, would immediately trigger "loop limit exceeded" error.

@dbaron
Copy link
Author

dbaron commented Jul 5, 2018

The CSS Working Group discussed this issue today.

@eeeps
Copy link

eeeps commented Oct 24, 2018

Sounds like this was discussed at TPAC yesterday: https://www.w3.org/wiki/TPAC/2018/SessionIdeas#ResizeObserver_extensions

Any updates worth sharing, @atotic @gregwhitworth ?

@gregwhitworth
Copy link

gregwhitworth commented Oct 24, 2018 via email

@atotic
Copy link
Collaborator

atotic commented Dec 8, 2018

Discussion moved to csswg: w3c/csswg-drafts#3326

@atotic atotic closed this as completed Dec 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants