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

Phenogrid: Add a scroll bar #363

Closed
glass-ships opened this issue Oct 3, 2023 · 0 comments · Fixed by #372
Closed

Phenogrid: Add a scroll bar #363

glass-ships opened this issue Oct 3, 2023 · 0 comments · Fixed by #372
Assignees

Comments

@glass-ships
Copy link
Collaborator

glass-ships commented Oct 3, 2023

Larger compares extend beyond the screen, and can scroll, but the scroll bar is at the bottom and not very obvious / doesn't persist unless you're hovering over

@glass-ships glass-ships added this to the 2023-10 Release milestone Oct 3, 2023
kevinschaper pushed a commit that referenced this issue Oct 6, 2023
Closes #363 

- allow all non-mocked requests to pass through msw. the list of
exceptions was just getting to be too long and too hard to predict (e.g.
hard to know what format vite will compile assets into in which
contexts). the penalty will be once in a while a live api will be hit
during mocked testing; probably will only happen when adding new
endpoints.
- replace sem sim compare fixture data with bigger example (for stress
testing phenogrid)
- update sem sim compare api func to derive some additional data, like
totaling up the scores for each col and row
- rearrange css z-indices
- get rid of big arrows in table scroll
- force scrollbars to be visible on macos chrome and safari (not
firefox, not ios). more discussion on this below. add ability to add
little dark shadow on scrollable elements. previously was just using an
alpha mask, so it would just fade into the background, so sometimes
you'd miss it if the gradient landed in-between elements. being a gray
gradient will make it always visible on light/dark bgs.
- phenogrid component: add scrolling, sticky col/row headers, info
popup, download image
- add util funcs to support the above

Discussions:

Maybe someone can provide me here with some content to put in the info
tooltip per #362

I added a sorting method where the rows/cols get sorted based on the sum
of their cells' scores. I'm not sure if this is useful, but it kind a
seems like it to me because it basically puts the darker (higher score)
cells closer to the top left corner. Does anyone know what's involved
with doing the freq/rarity sorting and different calc methods here:

<img
src="https://user-images.githubusercontent.com/8326331/272598176-fdff68d8-a396-4139-8de0-607b14df3d03.png"
width="200" />

We can do phenogrid in two ways: HTML (table) or SVG, each with pros and
cons. A table is automatically (fairly) accessible, svg takes much more
work to make accessible (or we can just ignore this as an enhancement).
A table allows for "sticky" col/row headers more easily with CSS, with
svg this would necessitate javascript. A table is harder and much to
download as an image (taking a screenshot of the dom is brittle and
prone to mysterious cross-browser/device errors, e.g. safari just fails
to download when the grid has too many elements), with svg all you do is
download the source as a text file. A table requires setting background
colors on several elements to make the "stick" col/row headers work
(i.e. the top row has to have a white bg so when you scroll up and the
cells go under it, they don't pop through), an svg has clip paths and
masks that can "cut out" part of a shape without needing a background
color, keeping things transparent (good for image export, and doesn't
necessitate changing all the bg colors if you're using the widget on a
site with a dark bg for example). In the next PR, I'm going to start
trying to convert this to an SVG to get a better feel for the tradeoffs.

Regarding scrollbars. I didn't like the bulky arrows with the animations
on the tables. Looks clunky, but more importantly, it's not "simple".
Have to add new dom elements and restructure your component hierarchy to
accommodate this. Instead, I added a utility class that adds a shadow
that can simply be slapped on any element without changing anything
else, however it can only work in one direction at a time (e.g.,
couldn't have a horizontal and vertical scrolling together), but I think
that's an okay limitation. Also, I've added a utility class to force the
scrollbar to always be visible, but it still won't work on macos firefox
or ios. This is because apple has forced a design standard on ios where
the scrollbar hides unless you are actively scrolling (I guess to make
it look more slick and/or save screen space), and firefox has taken it
further and completely ignores attempts to force scrollbar visibility on
any platform (and instead just respect whatever the OS tells it to do).
You can change this behavior in your OS settings, and we can recommend
users do it, but there is _no (good) way to us to force it_. There are
libraries like Perfect Scrollbar that can force scrollbars on any
platform, but they are not good to use imo: they require adding new DOM
elements to draw the scrollbar (despite them often claiming they don't
affect the dom), and they basically (poorly) replicate the browser's
native scrolling behavior with javascript. Sounds easy, but there are
more things to account for than you'd think (speed, acceleration,
snapping, wheel vs touch vs trackpad vs keyboard control, etc.), and if
you check out Perfect Scrollbar for example, you can notice their
website's scrolling feels a bit off from native behavior. All of this is
to say, the solutions I've implemented here are as far as I'm willing to
go. We should leave it here, with CSS properties and some basic shadows.
If we still get these common complaints about scrollbars not being
visible, e.g. in ios, we will have to just say it's the browser's/OS's
fault for enforcing this bad design decision on us. This is the approach
stackoverflow seems to take, for example, with its scrollbars in big
code blocks. You will see they disappear on firefox. Going further is
just too brittle and not within our scope to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants