Page MenuHomePhabricator

Stop using JX.Scrollbar for main page content
ClosedPublic

Authored by epriestley on May 10 2015, 5:13 PM.

Details

Summary

Ref T8151. This is option (5). It needs a few adjustments but feels pretty good. Major issues are:

  • Without a mouse, the scrollbars overlap by default, so we must move the column off the right margin.
  • Scrolling sometimes "bleeds" between the chat vs the main frame in a way that's not as discrete as the old framed content, but feels generally reasonable to me.

If we pursue this, I'd plan to make these additional changes:

  • Move the panel away from the right margin only if the page scrollbars are zero-width (i.e., in OSX trackpad mode).
  • Fix the notch in the upper right corner when the chat is moved away from the right margin.
  • Probably remove the body "overflow-y: scroll" on Conpherence and Workboards.
  • Update the resizing code to deal with 300px vs 315px widths.
  • We can probably clean up some JX.Scrollbar "main panel" code.

Here's the "bad" case, where I've visually separated the column to provide room for a scrollbar. This isn't ideal, but looks and feels OK to me:

visual-margin.png (1×485 px, 92 KB)

Test Plan
  • Tried Firefox, Chrome, Safari, with and without a mouse.
  • Tried normal Conpherence.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Stop using JX.Scrollbar for main page content.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: btrahan, chad.

This is what it looks like on windows + chrome:

pasted_file (873×359 px, 36 KB)

Also,

plustwo

  • Snug up column with a mouse connected.
  • Fix notch.
  • Update resizing code.

There's a bit more tweaking available, but I think this version is shippable if the double-scrollbar compromises are generally agreed to be the best tradeoff here.

btrahan edited edge metadata.

I'm supportive but sad about it; see T8151#113322.

In terms of landing, perhaps we should wait until @chad has a chance to weigh in, and / or add some design feedback for this approach?

webroot/rsrc/js/application/conpherence/behavior-durable-column.js
77

whats "!!" do? not sure if typo or some clever casting of "0" to false and "some integer" to true or perhaps some other JS magic.

This revision is now accepted and ready to land.May 11 2015, 6:44 PM

!! is a vaguely-idiomatic (bool) cast in JS, since JS doesn't have casts. The first ! converts it to a boolean (but with the opposite value) and the other one converts it back to the original value.

You're welcome to land whatever people like best. I'll gripe if anything seems untenable.

Sounds good. We can revisit framing content again later, too (say, after Chrome gets a fix for T7740, hopefully). Or this might have a bunch of problems I haven't anticipated and push us back up the list. I'll push secure in a sec.

This revision was automatically updated to reflect the committed changes.