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.

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

feat: responsive nav #877

Merged
merged 4 commits into from
Nov 6, 2018
Merged

feat: responsive nav #877

merged 4 commits into from
Nov 6, 2018

Conversation

olizilla
Copy link
Member

@olizilla olizilla commented Nov 2, 2018 •
edited
Loading

This is a minimal implementation of a responsive nav as part of #871

We can go fancier in the future, but this is a minimal change to behave reasonably on small screens.

webui-responsive-navbar

On small screens, the navbar has a horizontal scroll, so you can swipe across to see the other nav items.

@olizilla
wip: exploring a responsive navbar
9578c51 
See #871

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
feat: navbar is repsonsive
28801da 
WIP on #871

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@ghost ghost assigned olizilla Nov 2, 2018
@ghost ghost added the status/in-progress In progress label Nov 2, 2018
@olizilla
chore: delint
d890165 
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@lidel
Copy link
Member

lidel commented Nov 2, 2018 •
edited
Loading

It looks AMAZING @olizilla ! 💖

Small nitpick:

Firefox for Android fades out the horizontal scrollbar quite a lot and user may not notice there are additional items on the right:

screenshot_2018-11-02 screenshot_20181102-132155 png png image 1080 x 1920 pixels - scaled 70

I wonder if we could add additional hint that there is a horizontal scroll. Like < or … on the right (or something else).

@olizilla olizilla changed the title Responsive nav feat: responsive nav Nov 2, 2018
@lidel
Copy link
Member

lidel commented Nov 2, 2018 •
edited
Loading

Switched to desktop and noticed that on my linux box there is a strange "gap" that was not present before:

v2.1.1 this PR
2018-11-02--23-07-14 2018-11-02--23-05-24

Is anyone else able to reproduce?

@olizilla
Copy link
Member Author

olizilla commented Nov 5, 2018

@lidel wat is that bar. Can you inspect it and give me any more info? I don't see it on Firefox or Chrome on mac.

@fsdiogo
Copy link
Contributor

fsdiogo commented Nov 5, 2018

I can't reproduce it either (macOS).

fsdiogo
fsdiogo approved these changes Nov 5, 2018
View reviewed changes
Copy link
Contributor

@fsdiogo fsdiogo left a comment

Choose a reason for hiding this comment

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

LGTM

lidel
lidel reviewed Nov 5, 2018
View reviewed changes
Copy link
Member

@lidel lidel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ok, I the grey bar is.. scrollbar 🙃
I quess on macOS it has transparent background that is why you can't see it.

It disappeared when I removed overflow-x-scroll class:

2018-11-05--16 09 07

2018-11-05--16-10-32

I wonder how it looks on MS Windows.
@hacdias are you able to check this branch?

@olizilla perhaps overflow-x-scroll class should be added only in narrow contexts?

@fsdiogo
Copy link
Contributor

fsdiogo commented Nov 5, 2018

Yup, adding the class overflow-x-hidden-l fixes that.

I was able to reproduce that on a mac applying this settings (General):

scroll

@fsdiogo
Copy link
Contributor

fsdiogo commented Nov 5, 2018

And a wild pokemon appears:

poke

@ghost ghost assigned fsdiogo Nov 5, 2018
@fsdiogo
Copy link
Contributor

fsdiogo commented Nov 5, 2018

Desktop

2

Mobile

1

@olizilla
Copy link
Member Author

olizilla commented Nov 5, 2018

@lidel the last commit from @fsdiogo should fix the scrollbar on linux, can you eyeball!

@olizilla olizilla merged commit 208db83 into master Nov 6, 2018
@ghost ghost removed the status/in-progress In progress label Nov 6, 2018
@olizilla olizilla deleted the responsive-nav branch November 6, 2018 11:58
lidel
lidel reviewed Nov 6, 2018
View reviewed changes
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Yes, that fixed it 👍 :)

@fsdiogo fsdiogo mentioned this pull request Nov 6, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel left review comments

@fsdiogo fsdiogo fsdiogo approved these changes

Assignees

@olizilla olizilla

@fsdiogo fsdiogo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@olizilla @lidel @fsdiogo