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

Mobile experience improvements #4694

Merged
merged 8 commits into from
Mar 4, 2020
Merged

Mobile experience improvements #4694

merged 8 commits into from
Mar 4, 2020

Conversation

gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented Feb 29, 2020

What type of PR is this? (check all applicable)

  • Other

Description

Improvements on Redash experience for mobile devices.

Improvements:

  • Dashboard page
    • Re-enable touch (React Draggable disables it by default)
    • Improve Plotly Touch behavior
  • Remove auto focus from ItemList wrapper
  • Adjust spacing in bottom between Favorite Dashboards and Favorite Queries on Home page
  • System status tabs are creating horizontal scroll on mobile
  • Improve TagsList for mobile (not so good for several tags, and add support to multi-select with the touch - this one may deserve a proper UI remake)
  • Update Dashboard and Alert page headers mobile behavior with the Query page one

I'll probably break each of those into new PRs not to block improvements in v9.

Related Tickets & Documents

#2796

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Dashboard scrolling + Plotly
dashboard-scrolling

Adjust spacing in Home page

Before After
image image

Admin pages max-width

Before After
admin-page-before admin-page-after

@gabrieldutra gabrieldutra self-assigned this Feb 29, 2020
@gabrieldutra
Copy link
Member Author

@arikfr (cc @kravets-levko) I discovered an issue with React Grid Layout/React Draggable (react-grid-layout/react-grid-layout#637 which is closed, but not solved), basically they are disabling touch with touch-active: none. Added an workaround in this PR and it seems to have already improved mobile experience on dashboards substantially.

A further improvement we can think in there for Plotly is to change the Drag Zoom behavior, I came up with a few possible options:

  1. Render a static plot on mobile, solves the problem, but we lose Plotly's potential;
  2. Change default drag behavior to be the Move tool instead of Zoom. Can be done for desktop and mobile or for mobile only. My personal opinion is that the Move tool is the one I expect, but I'm not sure what's more convenient for others.
  3. Leave as is - it can be that being able to scroll outside of the Chart is enough improvement

Personally, I'd go with 2 by changing the default tool to a Pan at least for mobiles. There's no cursor to indicate that is a Zooming behavior, so I guess the moving would be expected.

@arikfr
Copy link
Member

arikfr commented Mar 1, 2020

Do you know why they disable touch?

Re. Plotly: I've tried switching to the Move tool but it still captured my interactions. The best solution would be that you had to actively click on the chart to enable interaction with it. Does Plotly support something like this?

@gabrieldutra
Copy link
Member Author

Do you know why they disable touch?

Almost sure is for the drag handle to work properly on resizing (or for moving grid items). I kept it disabled when the dragging is active, but on mobile it shouldn't affect anything. Also it seems it should work on iPads regardless of disabling the touch (I guess most cases where one could want to use a touch interface to edit a dashboard is for tablets).

As for plotly I haven't found any options like this yet, but I'll see if I can come out with some ideas on this behavior.

@arikfr
Copy link
Member

arikfr commented Mar 1, 2020

As for plotly I haven't found any options like this yet, but I'll see if I can come out with some ideas on this behavior.

One ugly workaround is to have a div on top of the chart, that will catch the touch events. But then we lose the interactivity of the chart.

@arikfr arikfr added this to the v9 milestone Mar 1, 2020
@gabrieldutra
Copy link
Member Author

gabrieldutra commented Mar 2, 2020

One ugly workaround is to have a div on top of the chart, that will catch the touch events. But then we lose the interactivity of the chart.

Pushed a test commit (0d65f2a), I think it's possible to evolve it with a toggle like the one you mentioned to be necessary to click first (we can explore UI options for it if you think that's the way to go).

Other options that occurred to me were to disable it for specific tools (zoom for example) and also Plotly v1.43.0 has introduced dragmode: false, which makes it possible to default a Chart with no tool selected (although there's no way to deselect a tool after selecting one). For this last one I need to confirm that there are no issues on upgrading as the current version is pinned to v1.41.3. If there are no issues with this small upgrade, I think the quick win option is to have no default tool selected for touch devices.

@gabrieldutra gabrieldutra marked this pull request as ready for review March 3, 2020 20:24
@gabrieldutra
Copy link
Member Author

gabrieldutra commented Mar 3, 2020

@arikfr, those are the improvements I came up with (all quick wins are implemented):

  • Dashboard page
    • Re-enable touch (React Draggable disables it by default)
    • Improve Plotly Touch behavior (currently updated from v.1.41.3 to v1.43.2)
  • Remove auto focus from ItemList wrapper
  • Adjust spacing in bottom between Favorite Dashboards and Favorite Queries on Home page
  • System status tabs are creating horizontal scroll on mobile
  • Improve TagsList for mobile (not so good for several tags, and add support to multi-select with the touch - this one may deserve a proper UI remake)
  • Update Dashboard and Alert page headers mobile behavior with the Query page one

For the last 2, the TagsList should be better alongside with a series of improvements for Desktop as well (better UI for multiple tags, hide tags when selected, better multi-selection, etc). As for the Dashboard and Alert page headers, the Query one recently had an update to wrap both tags and control buttons, if you think it's worth it I can update them. The dashboard one currently hides a lot of things on the mobile version as the way to solve it.

@arikfr arikfr merged commit e0312fb into master Mar 4, 2020
@arikfr arikfr deleted the mobile-improvements branch March 4, 2020 10:55
@arikfr
Copy link
Member

arikfr commented Mar 4, 2020

I merged this one as it stands on its own.

Let's do the Dashboard/Alert page header mobile view now and then discuss how we want to address tags.

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 this pull request may close these issues.

2 participants