Page MenuHomePhabricator

Add a toggle, sticky pref, for Conpherence Widget Pane
ClosedPublic

Authored by chad on Sep 9 2016, 8:04 PM.
Tags
None
Referenced Files
F12806312: D16533.id.diff
Wed, Mar 27, 6:06 PM
Unknown Object (File)
Tue, Mar 19, 10:24 PM
Unknown Object (File)
Tue, Mar 19, 9:07 PM
Unknown Object (File)
Tue, Mar 19, 8:49 PM
Unknown Object (File)
Tue, Mar 19, 7:35 PM
Unknown Object (File)
Tue, Mar 19, 7:20 PM
Unknown Object (File)
Tue, Mar 19, 6:43 PM
Unknown Object (File)
Tue, Mar 19, 6:26 PM
Subscribers

Details

Summary

This adds a "column" icon into crumbs, like in workboards, for expanding or hiding the "Widget Pane". This is per user sticky and defaults to off.

Test Plan

View a Conpherence Room, see no widgets by default. Toggle it on, see widget. Reload page, see widget stick. Verify mobile, tablets ignore hiding.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Add a toggle, sticky pref, for Conpherence Widget Pane.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.

jx-viewport isn't super happy here either.

I don't know how to javascript, since this doesn't work, but I think the ideas are right?

chad retitled this revision from Add a toggle, sticky pref, for Conpherence Widget Pane to [Draft] Add a toggle, sticky pref, for Conpherence Widget Pane.Sep 9 2016, 9:27 PM
chad retitled this revision from [Draft] Add a toggle, sticky pref, for Conpherence Widget Pane to Add a toggle, sticky pref, for Conpherence Widget Pane.Sep 10 2016, 3:57 AM
chad updated this object.
epriestley edited edge metadata.

Defaulting to off seems a little weird to me, but we can give it a shot.

Slight simplification/bugfix inline but this is basically fine.

webroot/rsrc/js/application/conpherence/behavior-toggle-widget.js
15โ€“18

Instead of doing this classname test, just do:

config.show = !config.show;
JX.DOM.alterClass(node, 'hide-widgets', config.show);
21

As written, this does the wrong thing if you click twice (or any other even number of times), since we never change config.show. That is:

  • Show column.
  • Reload page: column visible.
  • Click button twice: column hides, then visible again.
  • Reload page.
  • Expect: column visible.
  • Actual: column hidden, because this button always saves "the state opposite the state that we had when the page was loaded" when clicked.
This revision now requires changes to proceed.Sep 12 2016, 10:53 PM

(Fixing the classname test should also fix the other bug.)

chad edited edge metadata.
  • fix layout bugs, hide options on mobile
webroot/rsrc/js/application/conpherence/behavior-toggle-widget.js
15โ€“18

having trouble with it taking two clicks to toggle the first load. I think thats why I did the check, to read it right off the node.

webroot/rsrc/js/application/conpherence/behavior-toggle-widget.js
21

Does this need to be flipped, since you're flipping config.show before getting here?

webroot/rsrc/js/application/conpherence/behavior-toggle-widget.js
21

If I flip it, still takes two clicks to show or hide the column, however we send two setting requests so then it seems broken.

chad edited edge metadata.
  • update per comments
webroot/rsrc/js/application/conpherence/behavior-toggle-widget.js
17

Oh, sorry -- this one needs to be flipped. Seems to work correctly to me with this change to add an !:

- JX.DOM.alterClass(node, 'hide-widgets', config.show);
+ JX.DOM.alterClass(node, 'hide-widgets', !config.show);

seems to be a lag issue for me. if I wait 5 seconds, works fine, otherwise seems to take two clicks

epriestley edited edge metadata.

I can't reproduce any issues with it locally.

This revision is now accepted and ready to land.Sep 12 2016, 11:18 PM
This revision was automatically updated to reflect the committed changes.

pasted_file (997ร—1 px, 180 KB)

The side-panel is sticky, but the main pane isn't resizing when ticking it on or off...

(If starting off and then enabling it, it hides some of the text).

On the plus side, the native scrollbar is back!

(Chrome 52/Windows)

Not sure how to reproduce this.

I just toggled the thing on, reloaded the page, and switched it off again.

I can fire up windows later tonight and see. Can't reproduce on Mac. It's just adding and removing a css class.

I saw this one time in Safari/Mac, but it went away after a reload and I can't reproduce it any more. I feel like this is bad cache, but no idea why with celerity. Can't repro Chrome/FF/Safari Mac or Windows.

Still happening to me, even after a "clear cache and hard reload".
I can also see it on MSIE when logged out - turning the side-panel on hides some of the text:

pasted_file (221ร—133 px, 15 KB)