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

Move text on button press #44

Merged
merged 3 commits into from
May 17, 2020
Merged

Move text on button press #44

merged 3 commits into from
May 17, 2020

Conversation

fstr0
Copy link
Contributor

@fstr0 fstr0 commented Apr 23, 2020

Modify padding on active button to move the text 1px down and 1px right to replicate behaviour in Win98.

This fixes issue #34.

build/98.css Outdated
@@ -102,6 +102,7 @@ button:not(:disabled):active {
box-shadow: inset -1px -1px #ffffff,
inset 1px 1px #0a0a0a, inset -2px -2px #dfdfdf,
inset 2px 2px #808080;
padding: 2px 11px 0 13px;
Copy link

@DiskJunky DiskJunky Apr 23, 2020

Choose a reason for hiding this comment

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

I had the exact same fix. Just to note for posterity on some of the idiosyncrasies of the fix:

  • 2px was required due to Chrome not moving a 1px value. There appears to be a conflicting rule in css resolution there
  • 0px for the bottom as negative values aren't valid in FireFox and Chrome for padding. Interestingly, full padding-top, right, etc. does actually work with a negative value. Issue is specifically with the condensed padding tag and negative values for both browsers.

@jdan
Copy link
Owner

jdan commented Apr 25, 2020

Is there a way to do this without padding? I worry about someone adding their own padding (for larger buttons) and forgetting to add/subtract one on :active

Copy link
Owner

@jdan jdan left a comment

Choose a reason for hiding this comment

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

Padding Q

@tpenguinltg
Copy link

Perhaps you can make use of an invisible border instead since box-sizing is set to border-box? As far as I can tell, the border has been removed on buttons anyway, and I don't see any legitimate use for a downstream user to add it back if they want to keep the theme.

@michaeldll
Copy link

Perhaps you can make use of an invisible border instead since box-sizing is set to border-box? As far as I can tell, the border has been removed on buttons anyway, and I don't see any legitimate use for a downstream user to add it back if they want to keep the theme.

I had a go at implementing your suggestion on #79 :)

@jdan
Copy link
Owner

jdan commented May 4, 2020

@fstr0 I'm happy to merge this as-is, since I don't think we can do better than adjusting the padding :) Wanna rebase off master?

@vercel
Copy link

vercel bot commented May 5, 2020

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/jdan/98css/1m4i1tq0b
✅ Preview: https://98css-git-fork-fstr0-master.jdan.now.sh

@fstr0
Copy link
Contributor Author

fstr0 commented May 5, 2020

@jdan I think that is done now? I'm a GitHub noob so let me know if I did it wrong

Copy link
Owner

@jdan jdan left a comment

Choose a reason for hiding this comment

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

Hey @fstr0,

Sorry for the pain! We've removed build/98.css and docs/98.css from version control, so they'll need to be git rmd before I merge.

build/98.css Outdated Show resolved Hide resolved
docs/98.css Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview May 8, 2020 18:02 Inactive
@jdan
Copy link
Owner

jdan commented May 17, 2020

@fstr0 thanks so much <3

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.

5 participants