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

[resize-observer] Make all returned properties have the word box in them #3549

Open
gregwhitworth opened this issue Jan 23, 2019 · 12 comments
Open

Comments

@gregwhitworth
Copy link
Contributor

To be very clear, here are the boxes whos dimensions would be returned:

  • borderBoxSize
  • contentSize
  • scrollSize
  • devicePixelBorderBoxSize

If we can observe border-box, content-box, scroll-box and device-pixel-border-box, shouldn't all property names also include box in the name?

  • borderBoxSize
  • contentBoxSize
  • scrollBoxSize
  • devicePixelBorderBoxSize

Originally posted by @TremayneChrist in #3329 (comment)

@gregwhitworth
Copy link
Contributor Author

This is created to track the change for updating the names to be consistent

@atotic
Copy link
Contributor

atotic commented Jan 23, 2019

I prefer to report size to box. Box is often synonymous to rectangle, and rectangles have location + size by convention. We are only reporting width/height, therefore size is preferable for reporting.

Why are we using box instead of size in ResizeObserver::observe? There was only a weak preference for 'box' over 'size', this was an open issue in the spec for a while.

Box stands for CSS box model. Size was already part of ResizeObserver name, so it felt redundant.

@gregwhitworth
Copy link
Contributor Author

gregwhitworth commented Feb 21, 2019

Resolution of this issue depends on how we resolve on #3550 and #3329

@gregwhitworth
Copy link
Contributor Author

I agree with @atotic taking into account the current information we're returning, that said we mentioned many times regarding rect based information that it's "easy to add" it in. Do we want to leave it open to potentially support offsets or scope to what is today, because if we add in the offsets having size will be confusing as well.

@TremayneChrist
Copy link
Member

Based on the new shape, borderBox and contentBox will be returned.

As this is still an open issue, could this still change?

@gregwhitworth
Copy link
Contributor Author

@TremayneChrist based on the opinions of people, if we say Box that implies that you should be able to produce the rect when taking into account the context of other APIs such as getBoundingClientRect() authors may expect a box to have all of the same information. Size is more accurate for what's being returned now. There's also an argument to be made for authors asking to observer content-box and then getting contentSize which is also somewhat confusing even if the name itself is more representative of the content within.

What are your thoughts, which name would you want?

@TremayneChrist
Copy link
Member

@gregwhitworth now that the API adds support for future fragments and no longer returns a basic size object, there's potential to bring in consistency with the naming and reduce the confusion. For me, it feels more natural to watch content-box and then receive contentBox. Later on there may also be the need to add inlineOffset and blockOffset to the fragment information, which makes Size even more redundant.

@bkardell
Copy link
Contributor

It's tough to balance clarity, convenience and consistency in the platform APIs. As @atotic said though, ResizeObserver includes 'size' and each of the values is a ResizeObserverSize so 'size' is contextually inferred and including it suffixed on each name here feels perhaps unnecessarily verbose. Could it ever report more than sizes?

@gregwhitworth
Copy link
Contributor Author

gregwhitworth commented Mar 11, 2019

@bkardell It could thearetically but then we would need to observe those as well. Personally, I think the name being "Resize" implies that you're watching and receiving sizes. I personally tend to agree with @TremayneChrist as well as I asked for content-box so seeing contentBox doesn't confuse me; especially with the context stated in the previous sentence. So that will result in this:

contentBox: [{
  inline: 400,
  block: 400   
}]

Thoughts?

@bkardell
Copy link
Contributor

@gregwhitworth I think I like that... Each of those objects would have a .target as well?

@gregwhitworth
Copy link
Contributor Author

@bkardell yes everything from #3329 would be reflected the same way just the name would be contentBox or borderBox like above and will have contentRect for legacy and target for the element that is being observed. @atotic what are your thoughts.

@gregwhitworth
Copy link
Contributor Author

To be very clear it would be:

entry = {
        target: <Element>,
        contentRect: {/* v1 for back compat */},
        contentBox: null,
        borderBox: [{
            inline: 400,
            block: 400
        }]
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
resize-observer
  
In progress
Development

No branches or pull requests

4 participants