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

improvement: browse libraries styles fixed #2694

Merged
merged 5 commits into from
Jan 2, 2021

Conversation

benjakugler96
Copy link
Contributor

I did some stylings changes in order to fix the "Brose Libraries" link button when the scrollbar appears.
The 17px padding is because of windows scroll width, so when the user see the scroll, its not on top of the link button.
You can appreciate in the gif below that when scrollbar expands in Mac OS, then the scrollbar is not on top of the link button.

browse-libraries

@vercel
Copy link

vercel bot commented Jan 1, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/hycw33vf4
✅ Preview: https://excalidraw-git-fork-benjakugler96-link-styles.excalidraw.now.sh

@dwelle
Copy link
Member

dwelle commented Jan 2, 2021

Thanks for the PR!

I tweaked the underlying JSX a bit, removing the <Stack> which seemed an overkill, plus it made styling harder. Removed row-reverse and moved the link below the buttons.

Removed the 17px padding — not sure if this was necessary for Mac's overlay scrollbar when you don't have a mouse connected. If so, we'll need to reintroduce. Can you check @benjakugler96?

@benjakugler96
Copy link
Contributor Author

@dwelle seems good! The 17px is because of windows scroll too. This is how it looks in windows with out it
Screen Shot 2021-01-02 at 10 28 41

@dwelle
Copy link
Member

dwelle commented Jan 2, 2021

That's weird, can't repro that:

image

I thought only Big Sur may be affected, which I can't test.

Anyway, I'll reintroduce the 17px for now, then.

@lipis
Copy link
Member

lipis commented Jan 2, 2021

can we make them 16px at least? 17 is prime :)

@dwelle
Copy link
Member

dwelle commented Jan 2, 2021

can we make them 16px at least? 17 is prime :)

17px is the standard width of scrollbars. I made it 18px.

src/components/LayerUI.scss Outdated Show resolved Hide resolved
@dwelle
Copy link
Member

dwelle commented Jan 2, 2021

Merging. Thanks @benjakugler96 ❤️

@dwelle dwelle merged commit 7c3513b into excalidraw:master Jan 2, 2021
@ad1992 ad1992 changed the title feat: browse libraries styles fixed Improvement: browse libraries styles fixed Jan 2, 2021
@ad1992 ad1992 changed the title Improvement: browse libraries styles fixed improvement: browse libraries styles fixed Jan 2, 2021
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.

Browse libraries link doesn't account for scrollable library
3 participants