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

Misc Component Cleanup #1044

Merged
merged 10 commits into from
Oct 1, 2019
Merged

Conversation

nummi
Copy link
Collaborator

@nummi nummi commented Sep 29, 2019

MainContent / MonitorContentHeight

See commits: df45d75 and acf985e

The new component name describes what it actually does.

X-App

See commit eb52384

This component existed to apply two classes: is-dragging and inactive. This is now handled in the application template.

Drag Handle

See commit 504e2fd for changed code.

We were doing a lot of passing around the Application controller and setIsDragging action just to keep the column drag cursor from being glitchy. I removed all of that and just expand the width of the drag column so the hover style (column cursor) stays in place.

(The column cursor is applied on hover of the drag handle. If you drag your cursor quickly to the left or right you are briefly not hovering until the UI catches up — this displays the default cursor. To fix this we were tracking isDragging at the application level, applying a class to the root to keep the column cursor. There was a lot of plumbing involved and I replaced it simply by increasing the width of the hover area on mousedown.)

Default width of column:

default

On mousedown:

mousedown

It's a little difficult to see in an animated GIF but here is the glitch without the CSS fix:

glitch

@nummi nummi force-pushed the misc-component-cleanup branch from 6581779 to 16649e7 Compare September 30, 2019 06:29
nummi added 9 commits October 1, 2019 11:18
`</>, this, @, etc.`
`</>, this, @, etc.`
`</>, this, @, etc.`
This seems to be covering an edge case that no longer exists
`</>, this, @, etc.`
isDragging was used to keep the col-resize cursor active
@nummi nummi force-pushed the misc-component-cleanup branch from 16649e7 to 504e2fd Compare October 1, 2019 18:19
@nummi nummi force-pushed the misc-component-cleanup branch from acf985e to 00351dd Compare October 1, 2019 18:37
@nummi nummi changed the title WIP Misc Component Cleanup Misc Component Cleanup Oct 1, 2019
@nummi nummi requested a review from RobbieTheWagner October 1, 2019 18:39
@RobbieTheWagner
Copy link
Member

This is 🔥 . Love removing all the controller injections!

@RobbieTheWagner RobbieTheWagner merged commit 1044293 into emberjs:master Oct 1, 2019
@nummi nummi deleted the misc-component-cleanup branch October 1, 2019 21:05
patricklx pushed a commit to patricklx/ember-inspector that referenced this pull request Sep 19, 2022
* <DraggableColumn>

`</>, this, @, etc.`

* <DragHandle>

`</>, this, @, etc.`

* Install element-modifiers

* <ResizableColumn>

`</>, this, @, etc.`

* Move DraggableColumn components under Ui

* Remove #setWidth from Ui::ResizableColumn

This seems to be covering an edge case that no longer exists

* <MainContent>

`</>, this, @, etc.`

* Remove {{x-app}}

* Remove isDragging from application controller

isDragging was used to keep the col-resize cursor active

* Rename component MainContent to MonitorContentHeight
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants