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

UI: document and viewport dom service additions #4994

Merged
merged 1 commit into from
Dec 4, 2018

Conversation

johncowen
Copy link
Contributor

window and document are easily injected, but this
primarily this keeps everything dom related in the same place.

(additionally see #4789 (comment) for more information on the dom service)

Included here are changes to make all ember related objects use the dom
service document and viewport instead of just document and
window.

Quote from a previous PR, #4924 (comment) and #4924 (comment), which explains the thinking around this:

Now I have all these things in the dom service, it would make sense
to get window from there also. I was thinking of making a viewport
method, which would be a nice word whether window was a browser window,
an iframe (not really a window) like when ember testing, or anything
else. To me the viewport is what we are actually talking about here.

`window` and `document` are easily injected anyhow, but this
primarily this keeps everything dom related in the same place.

Included here are changes to make all ember related objects use the dom
service `document` and `viewport` instead of just `document` and
`window`.

Quote from a previous PR (#4924) which explains the thinking around this:

> Now I have all these things in the dom service, it would make sense
to get window from there also. I was thinking of making a viewport
method, which would be a nice word whether window was a browser window,
an iframe (not really a window) like when ember testing, or anything
else. To me the viewport is what we are actually talking about here.
@johncowen johncowen requested a review from a team November 26, 2018 14:25
@johncowen johncowen added the theme/ui Anything related to the UI label Nov 26, 2018
@johncowen johncowen force-pushed the ui-staging branch 3 times, most recently from d244558 to 86624f0 Compare November 30, 2018 10:40
const event = get(this, 'dom').normalizeEvent(e, value);
const form = get(this, 'form');
Copy link
Member

Choose a reason for hiding this comment

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

👍

@johncowen johncowen changed the base branch from ui-staging to feature/ui-staging-temp December 4, 2018 17:12
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM

@johncowen johncowen merged commit 83756c1 into feature/ui-staging-temp Dec 4, 2018
@johncowen johncowen deleted the feature/ui-dom-additions branch December 4, 2018 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants