-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: add topbar title #21
Conversation
src/index.js
Outdated
position: absolute; | ||
top: 0; | ||
left: 0; | ||
width: 100%; | ||
z-index: 1; | ||
border-top-left-radius: 0.4em; | ||
border-top-right-radius: 0.4em; | ||
background-clip: padding-box; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 37-44 were just re-indented - see whitespace insensitive diff: https://github.com/coston/react-window-ui/pull/21/files?diff=split&w=1
border-top-left-radius: 0.4em; | ||
border-top-right-radius: 0.4em; | ||
background-clip: padding-box; | ||
font-family: -apple-system, BlinkMacSystemFont, Helvetica, Arial, sans-serif, "Apple Color Emoji"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the system font stack from https://css-tricks.com/snippets/css/system-font-stack/ but only the Mac ones.
Would really love to see this feature in! |
@Darrekt @AndersDJohnson Hey y'all! I'm sorry for the delay. This slipped my radar. This is a great enhancement of the component. 🙌 Improvements that I think should be made based on the screenshot:
Please look into the details above, if you have capacity. A personal blocker is that this PR is hard to quickly inspect, as I have little capacity myself. I am going to drop in a dedicated demo site within the next few days. I will merge this feature by the end of the week. |
@AndersDJohnson @Darrekt this seems to work well. None of the spacing issues seen in the PR screenshot. Great job, @AndersDJohnson 🎉 |
This was published at |
Thanks much, @coston! 💯 I understand the struggle of maintaining small open source packages and feature requests from random people... 😅 |
Adds support for optional topbar titles using CSS
content
. AddtopbarTitle
prop (didn't usetitle
since it becomes the HTMLtitle
attribute).Also switching the units from
em
torem
since otherwise things might not line up (especially with #22) since a different font is applied to the topbar vs. the content which inherits from the document.Here it is for all three versions (
Terminal
,MacTerminal
,Browser
):Fixes #20.