Skip to content

Correct key widths #34

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MiniGod
Copy link
Contributor

@MiniGod MiniGod commented Mar 8, 2019

Currently issue: When giving keys a custom size it can be hard to make them align correctly with the other lines, and the math doesn't add up. For instance, in the page running in npm run dev, the 0 key for the numeric input is supposed to be 3x as big as a normal key. A normal key flex: 40, so logically, the 0 key should be { size: 40 * 3 }, but then it is too small so it is set to 130. This is caused by the margins mixed with flex.

This PR fixes this so that the keys align correctly, and the math adds up. It is mostly just code around, and had to wrap all keys in another div.

I tried to keep the diff clean, but I had to indent so I suggest you look at the diff with "No Whitespace" enabled.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 5.217% when pulling 4f65a07 on MiniGod:correct-key-widths into 8bfd533 on icebob:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 5.217% when pulling 4f65a07 on MiniGod:correct-key-widths into 8bfd533 on icebob:master.

@coveralls
Copy link

coveralls commented Mar 8, 2019

Coverage Status

Coverage remained the same at 5.042% when pulling c3afd0f on MiniGod:correct-key-widths into 8f37ccc on icebob:master.

@icebob
Copy link
Owner

icebob commented Jul 16, 2019

Could you show screenshot about differences, I mean how it looks before & after PR.

@MiniGod
Copy link
Contributor Author

MiniGod commented Jul 16, 2019

Of course!

master:
master-130

this pr:
pr-120

I just pushed a fix to set the width of zero so it is align with the other buttons. Width of zero is now 120 because it should be 3 times as big as the other buttons on that row (which have default width of 40).

Before this PR the width of zero was 130 to align with the other buttons, which must have been found by trial and error, rather than math, which is the point of this PR to solve.

For your convenience, here is a screenshot of this PR without changing the size of zero:
pr-130

Notice how the zero is too big. This might be considered a breaking change by some if they had custom layouts with width values to align with other buttons.

@MiniGod MiniGod force-pushed the correct-key-widths branch from b0e388b to c3afd0f Compare July 16, 2019 17:24
@MiniGod
Copy link
Contributor Author

MiniGod commented Jul 16, 2019

And this is how the normal keyboard changed.

Note: I have not changed any of the width properties of the buttons in the normal keyboard

master:
master-normal

this pr:
pr-normal

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.

3 participants