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

Use CSS to stop greyed Right/LeftPanel UI from being interactable #5422

Merged
merged 3 commits into from Oct 25, 2017

Conversation

@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Oct 24, 2017

Fixes #2073

Copy link
Member

dbkr left a comment

argh, I dislike asking people to fix sins of the past in PR review but making a setting called 'opacity' control whether the element is disabled - we probably ought to change the prop to 'disabled' or something?

Also pointer-events doesn't feel like the best way to do this but I can't think of a better suggestion given that doing it in js would involve passing a disabled prop into every single child, recursively.

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Oct 24, 2017
@lukebarnard1

This comment has been minimized.

Copy link
Contributor Author

lukebarnard1 commented Oct 24, 2017

Yep, you're absolutely right. Being allowed this level of flexibility with opacities of the different panels is unnecessary imo. Let's have a disabled on LeftPanel and RightPanel that applies a CSS class that can do the opacity and pointer events (I think it's fine to do this with CSS tbh, so long as everyone is aware of what pointer-events does) and modify ui_opacity to be two different dispatches fade_side_panels and unfade_side_panels.

WDYT?

@t3chguy

This comment has been minimized.

Copy link
Collaborator

t3chguy commented Oct 24, 2017

Forwarding does some funky stuff with fading just the right panel and needing to leave access to the left panel as thats how you select the room to forward to
having no granularity in those dispatches doesn't sound ideal for that

@lukebarnard1

This comment has been minimized.

Copy link
Contributor Author

lukebarnard1 commented Oct 25, 2017

@t3chguy has a point there and some ui_opacity dispatches need to fade the middle panel whereas others don't. So I propose we keep the ability to fade panels independently but drop the opacity prop and we can use a mx_fadable_faded class instead.

@dbkr

This comment has been minimized.

Copy link
Member

dbkr commented Oct 25, 2017

yeah, this sounds like a winner then

Simplify the API for disabling panels in the UI. `mx_fadable_faded` is applied instead of setting opacity.
Part of simplifying use of mx_fadable
@dbkr
dbkr approved these changes Oct 25, 2017
@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Oct 25, 2017
@lukebarnard1 lukebarnard1 merged commit 7c7ae3a into develop Oct 25, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.