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

Change layout-rect to DOM-rect #9951

Closed
wants to merge 4 commits into from
Closed

Conversation

tiendt
Copy link
Contributor

@tiendt tiendt commented Jun 15, 2017

closes #6131

@jridgewell
Copy link
Contributor

closes #6361

I don't think that's the right issue number.

@lannka
Copy link
Contributor

lannka commented Jun 16, 2017

My original thinking was to use the native data type DOMRect, so we can get rid of the custom LayoutRectDef or DOMRectDef, and together with both data converters DomRectFromLayoutRect and layoutRectFromDomRect.

However, @zhouyx pointed out DOMRect is not supported in Chrome :-(, so my proposal wasn't feasible.

I saw this PR changed LayoutRectDef to DOMRectDef, which still adds a bit value so that we can get rid of the DomRectFromLayoutRect converter. But since we still introduced a custom data type, shall we keep the original name LayoutRectDef?

@zhouyx
Copy link
Contributor

zhouyx commented Jun 16, 2017

Honestly I don't like that.. but I can't find a better solution. Or there's no better solution when different browsers are following different standard.... 😭
I would change our LayoutRect to use ClientRect and only convert them to DOMRect when we output them. But it's basically the same as converting everything to DOMRect first....

Whatever we do here, we need to name the function and doc them well....

@tiendt
Copy link
Contributor Author

tiendt commented Jun 17, 2017

@lannka I'm sorry I misunderstood your proposal :( Should we discuss this further to make a proper change?

@lannka
Copy link
Contributor

lannka commented Jun 20, 2017

closing this, in favor of #10028

@lannka lannka closed this Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change data type LayoutRect to DOMRect
5 participants