Skip to content

🐛 FIX: various horizontal scrolling issues on mobile #2220

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 21, 2019
Merged

🐛 FIX: various horizontal scrolling issues on mobile #2220

merged 1 commit into from
Nov 21, 2019

Conversation

@violoncelloCH
Copy link
Member

No description provided.

@violoncelloCH violoncelloCH force-pushed the fix/2110/horizontal-scrolling-on-mobile branch from 54bd865 to 07284b3 Compare November 7, 2019 17:03
@skjnldsv skjnldsv mentioned this pull request Nov 7, 2019
Closed
19 tasks
@violoncelloCH violoncelloCH force-pushed the fix/2110/horizontal-scrolling-on-mobile branch from 07284b3 to 9041de4 Compare November 7, 2019 19:52
ChristophWurst
ChristophWurst reviewed Nov 8, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Run npm run lint:autofix :)

@violoncelloCH violoncelloCH force-pushed the fix/2110/horizontal-scrolling-on-mobile branch from 9041de4 to 683add8 Compare November 9, 2019 13:45
@violoncelloCH
Copy link
Member Author

Run npm run lint:autofix :)

done :)

ChristophWurst
ChristophWurst reviewed Nov 11, 2019
View reviewed changes
@ChristophWurst
Copy link
Member

How do I test this?

@violoncelloCH
🐛 FIX: various horizontal scrolling issues on mobile
9c76cf8 
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
@violoncelloCH violoncelloCH force-pushed the fix/2110/horizontal-scrolling-on-mobile branch from 683add8 to 9c76cf8 Compare November 11, 2019 20:46
@violoncelloCH
Copy link
Member Author

hmm, best would be to test this together with nextcloud/server#17843 and nextcloud-libraries/nextcloud-vue#711 because those two solve related problems... or what do you mean by "how"?

@violoncelloCH violoncelloCH mentioned this pull request Nov 11, 2019
Merged
@ChristophWurst
Copy link
Member

More like what is the behavior before and after this fix :)

If we need nextcloud-libraries/nextcloud-vue#711 then we should wait for the next release of that lib

@violoncelloCH
Copy link
Member Author

test it on narrow mobile layouts of 340px width to compare before and after...
it makes the composer fit into those 340px and and also the message list is optimized for mobile..
yeah maybe it makes sense to wait for that release; on the other hand it's more an additional improvement than a requirement for this PR

@violoncelloCH
Copy link
Member Author

the only "functional" change is that it makes sure to not display the "no message selected" section on mobile (together with nextcloud-libraries/nextcloud-vue#711 this uses the correct breakpoint)...

ChristophWurst
ChristophWurst approved these changes Nov 21, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Let's do this :)

@ChristophWurst ChristophWurst merged commit ec070a1 into master Nov 21, 2019
@ChristophWurst ChristophWurst deleted the fix/2110/horizontal-scrolling-on-mobile branch November 21, 2019 10:22
@ChristophWurst ChristophWurst added this to the 0.19.0 milestone Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

Assignees

No one assigned

Labels

3. to review design

Projects

None yet

Milestone

0.19.0

Development

Successfully merging this pull request may close these issues.

3 participants

@violoncelloCH @ChristophWurst