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

Add touch slideout menu for mobile #400

Merged
merged 1 commit into from Sep 30, 2016
Merged

Conversation

maxpoulin64
Copy link
Member

Implements #393 with a slightly modified version of the demo solution I had. The only problem with it that I'm aware of is that the transition is fixed, so if you're like me and swipe the slider really fast it feels laggy when you release the finger as the animation still plays at normal speed. I didn't feel like implementing the maths to support inertia, but if that's bugging too much people I will add it.

@maxpoulin64 maxpoulin64 added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Jun 12, 2016
@dgw
Copy link
Contributor

dgw commented Jun 12, 2016

I believe @astorije himself would say it's better to add this and improve it later if necessary than try to get it perfect before merging. 👍

@xPaw
Copy link
Member

xPaw commented Jun 12, 2016

Trying this on desktop and there's a bug with the menu being a little buggy:

  1. Resize the window so that the menu hides
  2. Open the menu
  3. Resize to higher resolution, the sidebar will now re-appear

It looks like it is not possible to open/close the menu while on the login window.


When you tap something in chat view (to close the menu), it still triggers the click on whatever you tapped (e.g. if you tap on a nick, sidebar closes, and it executes a /whois)

@xPaw xPaw added this to the Next Release milestone Jun 13, 2016
@maxpoulin64
Copy link
Member Author

@xPaw:

1- I semi-fixed it, but master does that as well in an even more silly way (at least for me). It's hard to fix due to the way we do the menu (we actually put the menu with left: -220px and slide the whole viewport 220px instead of changing the absolute position of the menu itself). I can disable animations when going into desktop mode because the menu is never animated, but I can't easily disable it for when we transit between desktop->mobile.

2- Good catch, fixed

3- Semi-fixed. Since every window have its own menu button for some reason, by the time we get the event to fire on #viewport it's already clicked so I can't do much better than disabling pointer events on .messages :/

@astorije
Copy link
Member

@maxpoulin64, could you rebase this with master please?

@maxpoulin64
Copy link
Member Author

Sure, will do tomorrow or saturday as soon as I get a chance.

@maxpoulin64
Copy link
Member Author

@astorije: done, pretty minor conflict.

Copy link
Member

@astorije astorije left a comment

Choose a reason for hiding this comment

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

We might want to switch to a dedicated dependency for that later, but it will do until then. 👍

@astorije
Copy link
Member

If @xPaw or @williamboman or @MaxLeiter want to give it a try, I think this is ready to go 🎉.

@williamboman
Copy link
Member

I ran this for quite a while before the first 2.0 rc and it worked great then. Gonna update my instance to run latest master + interesting PRs (including this) again.

@MaxLeiter
Copy link
Member

I really like the change, but it feels a little too fast. Could we make the transition a little longer?

@MaxLeiter
Copy link
Member

From IRC

21:28 +Max-P Hmm yeah, I made it quite fast because it otherwise feel a bit weird when doing a quick swipe
21:29 +Max-P I'll make it a bit slower, can't make it too slow but I certainly can make it slightly slower.
21:29 +Max-P It's not just a gesture, you can actually drag it however you want, that's why I had to make it that way
21:35 MaxLeiter well when you open sidebar and tap anywhere, it closes
21:36 MaxLeiter that felt too fast, the dragging part was fine, but I assume those are the same value?
21:36 +Max-P Yeah
21:36 +Max-P Basically what happens is after the gesture, the end result is calculated and it snaps to either open or closed
21:36 +Max-P So it's the same (CSS) animation that finishes it
21:36 +Max-P But yeah I'll probably split them into two, or calculate it in JS and set it in style

@astorije
Copy link
Member

I'm in favor of shipping this now and improving later. It's not like it's broken, just it could be slightly improved. What do you think @MaxLeiter?

@MaxLeiter
Copy link
Member

Fine with me, @astorije

@astorije astorije modified the milestones: 2.1.0, Next Release Sep 30, 2016
@astorije astorije merged commit ad4a79c into thelounge:master Sep 30, 2016
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
Add touch slideout menu for mobile
@maxpoulin64 maxpoulin64 deleted the slide branch February 5, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants