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

Touch dashboards #775

Merged
merged 24 commits into from May 18, 2020
Merged

Touch dashboards #775

merged 24 commits into from May 18, 2020

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented May 14, 2020

Discussion in #752

Changes

Enables better control of panel dragging. Still a work in progress.

Checklist

  • All querysets/queries filter by Team (if applicable)
  • Backend tests (if applicable)

@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 14, 2020 15:48 Inactive
@timgl timgl had a problem deploying to posthog-touch-dashboard-atjurw May 15, 2020 09:01 Failure
@timgl timgl had a problem deploying to posthog-touch-dashboard-atjurw May 15, 2020 10:22 Failure
@timgl timgl had a problem deploying to posthog-touch-dashboard-atjurw May 15, 2020 11:02 Failure
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 15, 2020 11:53 Inactive
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 15, 2020 12:17 Inactive
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 15, 2020 12:31 Inactive
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 15, 2020 12:36 Inactive
@mariusandra
Copy link
Collaborator Author

Finally ready!

Since it's virtually impossible to detect if you have an user with a mouse or with a touch device... and you shouldn't design differently for either case anyway, I ended up going another route. I believe the end result is even better.

Now in order to rearrange the dashboards you have two options:

  1. You can click the "rearrange" button to start rearranging...

screencast 2020-05-15 14-39-36

  1. You can long-press on any of the dashboard items to start rearranging...

screencast 2020-05-15 14-42-36

To stop rearranging, either 1) click the same "rearranging" button, 2) click the "black toast" that appears in the bottom, 3) click anywhere that's not a dashboard item, e.g. on an empty cell 4) click ESC or 5) click the browser back button.

I've been playing with it and it works really intuitively for me on both desktop and mobile modes (on my phone, didn't try on a tablet yet).

Still to do to have everything perfect:

  • Right now when you long press, you need to release your mouse/finger and then press again to start dragging. It would be great if it would just hook into the item directly and you could move it around. I'll probably need to patch react-draggable/react-resizable for this to work well

  • Resizing from every corner. I spent 3h last night fighting with react-grid-layout to support dragging from multiple corners, but in the end just got it working from the bottom, right and bottom-right corners. I used someone else's react-grid-layout PR for this, merged it with the latest master and submitted as another PR: Resizable handles on other corners v2 react-grid-layout/react-grid-layout#1211 . Until then I published a npm package @mariusandra/react-grid-layout, which includes this support.

Also in this PR I did a few other things:

  • Pinned dashboards are now shown directly in the sidebar (fixes Open dashboards by default #773)
  • I added <meta name="viewport" content="width=device-width, initial-scale=1.0"> to the HTML to have the app be the right size in mobile mode, instead of showing a zoomed out desktop interface.
  • I updated the top header ("new version" + "worker stats" + email) to look a bit better in responsive mode

@mariusandra mariusandra marked this pull request as ready for review May 15, 2020 12:50
@mariusandra mariusandra requested a review from timgl May 15, 2020 12:50
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 15, 2020 13:50 Inactive
@timgl
Copy link
Collaborator

timgl commented May 15, 2020

Annoyingly I don't think this is an improvement over what we currently have for dashboard

  • It's not intuitive to long-click panels I find
  • When long clicking, you then have to click a panel again to drag it
  • On my new MacBook Air the animation of the wiggling is super laggy on our default dashboard
  • It's quite hard to exit the wiggly stage, as I have to click somewhere else multiple times or move somewhere entirely.

I understand this is a much better solution on mobile but 90% of our traffic at the moment is desktop so I'm in favour of optimising for that.

Edit: Having the dashboards in the sidebar is v cool, it would maybe be worth putting them in a slightly different section from the rest of the links, maybe just at the bottom (or top!) with a <hr> between them and the rest

@mariusandra
Copy link
Collaborator Author

Hi, I see your points and I'm open for suggestions to improve it.

I'd totally like to have the dashboard start moving without having to do a second click/tap, but I ran into limitations with the libraries used and figured it to be a practical compromise for now. This can be fixed though.

Regarding the animation and performance. Obviously if it's lagging, we need an alternative solution.

I'd like to add a counterpoint to the 90% statement though.

The current dashboards are unusable on mobile. I've spent the last hour using app.posthog.com on my phone and the experience was terrible.

As a site owner, I like to often check my stats on my phone. I know people who check their dashboards on their phone before going to sleep every day.

I think it's crucial that posthog works well on a touch device... And that can't be an afterthought that we install later.

I however agree with you that the current version is a slight downgrade when used with a mouse or a touchpad. Long clicking is indeed not the most intuitive thing.

@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 16, 2020 08:23 Inactive
@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 16, 2020 08:28 Inactive
@mariusandra
Copy link
Collaborator Author

mariusandra commented May 16, 2020

Hi!

I added a divider between the pinned dashboard items in the sidebar and the rest:
image

Regarding the logic with dragging, after having a night to sleep over it, I still believe we should make the app work well on mobile. The fact that 10% of users use PostHog on mobile despite how bad it works is testament that mobile support is necessary. We shouldn't wait for customer complaints before starting to fix this. :)

I'd ideally love to keep the interface as is (drag to move always + resize from corner always) for desktop users who use a mouse with a scroll wheel or a two finger touchpad gesture to scroll the page, yet there is no way to reliably detect this. Browser detection won't work as there will be too many exceptions (e.g. maybe you're using a mac with a touchscreen monitor?). We can detect usage of features (e.g. a lot of "mousemove" or "mousewheel" events to indicate that there is a mouse/touchpad/trackpad), yet I'm fairly confident windows laptops with touchscreens also send them on touch for example (experience building apps with a colleague who used a Thinkpad with a touch screen).

On touchscreen windows devices, there's some extra gesture control on Chrome, where touching the screen is normally a click, yet if you touch and drag, it scrolls. That probably breaks down if you touch and drag on an element which listens to mousedown/mousemove/touchstart/etc events. Plus then there are iOS devices that require you to specify explicitly that it's a touchpoint, otherwise your listeners will drag the elements and the page will scroll as well. (We had these issues in pigeon-maps for example).

All this is to say that for the best UX between devices, we need to have two modes: dragging enabled and dragging disabled.

This "lock dragging mode" switch might be a feature request anyway at some point, as I expect there can be too many accidents if we allow moving things around for everyone. I can imagine frustrated CEOs thinking "why is this dashboard not how I left it" and then opening a ticket.

If you exclude 1) the long click functionality on desktop, 2) the problem with having to click a second time to actually drag then and 3) the wiggly effect that might be slow on some devices, that's what this PR does. There's a button to activate the "rearranging mode" here:

Screenshot 2020-05-16 at 10 31 23

And five ways to get out of it: 1) clicking the same button again, 2) clicking the "toast" in the bottom, 3) clicking ESC, 4) clicking on the background, 5) clicking the browser back button.

The wiggling is just a way to make it clear that you're in "rearranging" mode. If it's slow, we should find some alternative solution. One idea is to only make things wiggle below a certain resolution when we're rather certain it's a phone/tablet.... and to just overlay arrow icons over the panels on higher width screens.

The other thing I could do is to make things wiggle only after long pressing with "touchstart" events, excluding "mousedown". The thing is touchstarts also trigger mousedowns, but mousedowns don't trigger touchstarts, so on Android/iOS devices we can detect a long-tap. On windows touchscreen devices (where everything is a mouse) you will only get into the "rearranging" mode then when clicking the button. (Comment out lines 48-51 in useLongPress to try).

In any case, please let me know what you think of all of this and what solutions we could consider. It might be worth pooling more opinions as well.

@mariusandra
Copy link
Collaborator Author

Hey, I just found one other use case that needs dashboard dragging to be locked. I wanted to copy the last item on this list on app.posthog.com, to paste it in my browser:

Screenshot 2020-05-17 at 00 21 14

Unfortunately I couldn't. It started to drag the panel instead. So I had to manually type it.

@timgl
Copy link
Collaborator

timgl commented May 18, 2020

I've had another play with this this morning

  • I 100% agree now that the dashboard does need to look good on phones (so the meta tags and some of those other changes should definitely go in)
  • I don't think being able to re-arrange on mobile is important enough to have a much worse experience on desktop
    • We could either have dragging just on the title bar (this might solve everything) or disable dragging all together on small screens
  • I don't think the locking/unlocking is a good idea. It adds 3 clicks where 1 would do, and it's just so much more fun to land on a page and immediately start dragging stuff around. I demo'd this to someone the other day and it's just a visually cool thing
  • WRT the copying of text, I was thinking the 'drag-bar' component should just be the title bar, not the whole panel. That should solve the copying of text, and it might even solve the scrolling on mobile, as there much less surface that will trigger. Also worth adding a cursor: grab to make that obvious.

Again, I really appreciate how much effort you put into making this re-arranging work but I guess I've just fallen in love with how this currently works in master. I also think this will be less work than trying to to implement the detection method you talked about in your comment.

@mariusandra
Copy link
Collaborator Author

Hey, how about trying something like this?

  • We still keep a "lock dragging" mode, yet flip it from what is now in this PR. So instead of turning on a "rearrange" mode, you can enable "lock dragging" if you want to lock it.
  • We have some relatively dumb logic that by default "lock dragging" is off, but if the screen is below a certain resolution OR the user agent says "iOS" or "Android", we lock by default.

However, to be fair, I expect that over the life of the product, 99% of the time if you open a dashboard you have no intention to rearrange anything. Making sacrifices for the 99% to serve the 1% feels a bit too much like going into American politics... 😁

@timgl
Copy link
Collaborator

timgl commented May 18, 2020

I think that makes sense! Let's also try the dragging on title bar only, as I think that'll super easily solve the copying-text issue plus might make it easier on mobile too.

@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 18, 2020 15:52 Inactive
@mariusandra
Copy link
Collaborator Author

Hi @timgl , a new batch of updates is up!

I did the following changes now

  • there are two dragging modes: normal and wobbly
  • in normal mode you can drag from the bottom and right edges/corner to resize and the title bar to move panels
  • there's a button "lock dragging" / "dragging locked" that enables/disables the normal dragging mode
  • this normal dragging mode is enabled by default, except if navigator.userAgent contains /Android|iPhone|iPad|iPod/
  • there's only one reason for keeping the wobbly mode (other than the wobbling being cool) - imagine you're on your phone and the dashboard is many screens long... and you want to rearrange things on the last screen. An alternative approach is to make the dashboard header with the "(un)lock dragging" button stick to the top of the screen when scrolling down, so you could always click it, but this doesn't look so good IMO
  • to enable the wobbly mode you must long-touch on an item (long clicking with the mouse does nothing)
  • when in wobbly mode you can drag from anywhere to move the item, except the bottom and right edges/corner, which are used for resizing and are also a bit wider than in normal dragging mode
  • in wobbly mode we show the black toast that says "click here to stop rearranging", so you don't need to scroll back to the top of the screen to "lock dragging"
  • clicking ESC or on the background does nothing anymore, as wobbly mode can only be activated on touch devices now

I think that's it. There's just one thing that still can't be properly done: you can't select text on a touch device, as that will enable wobbly dragging mode. I'm a bit conflicted about it. I could add logic to not enable wobbly mode when you click on selectable text, yet often when I want to enable wobbly mode, I see that I start it by e.g. longpressing on the number in the middle of the pie chart.

Also, the idea to always enable the same type of dragging, yet make only the panel headers and edges/corners interactive breaks down quickly when you try to use it on a phone. While only 10-20% of the screen is a "minefield" this way, it's still way to easy to hit a drag target when casually scrolling down. Thus this won't work.

@mariusandra
Copy link
Collaborator Author

Hi @timgl , a few more thoughts and a few related changes.

I think moving the panels from the title is fine, yet I still somehow want to drag them from wherever on my computer. It's also not really apparent that the header is where you can start dragging from. There's only one indication for that: the mouse cursor turns to the <-> move arrows. So unless you hover over the invisible header boundary, you won't know you can move the panels.

Thus I made some changes:

  • Now you can again drag the panels in the "normal" dragging mode from anywhere.
  • If you move the mouse over the header it turns to a move cursor, indicating that moving is in fact possible
  • Elsewhere on a panel the cursor is the normal one, but you can still move the panel (it looked really weird to hover over the line graph points when the mouse was in the "move cursor" mode)
  • There's just one exception. You can't drag text on a table. This will not start dragging the panel, but will just select the text.
  • Same with long touching on a panel. It'll enable wobbly mode only if you long pressed on anything except a table. Then you'll be able to select text normally.

I hope this fixes most of the UX issues we had. Please let me know if there's anything more to do :).

Copy link
Collaborator

@timgl timgl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
This looks great! My only small comment would be that on mobile, it feels like there's quite a lot of space unused, that we could use to make the graphs a bit wider. Will leave up to you whether you want to change that or not

@timgl timgl temporarily deployed to posthog-touch-dashboard-atjurw May 18, 2020 18:47 Inactive
@mariusandra
Copy link
Collaborator Author

Hi, good point. I reduced that space now by approx 66.6667% on smaller screens. Merging!

@mariusandra mariusandra merged commit dc447dc into master May 18, 2020
@mariusandra mariusandra deleted the touch-dashboards branch May 18, 2020 18:48
@timgl
Copy link
Collaborator

timgl commented May 18, 2020

😄

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.

None yet

2 participants