Baseline border fix
Done
On low pixel density devices a 1.5px border would computed as 1px. If we use $input-border-thickness = 1.5px in the calculation and the user is on a low pixel density screen then the border is rendered as 1px, but we still eg. subtract 1.5px from the padding which will throw off the baseline grid alignment. So we have to check if the users screen is capable of displaying 0.5px steps at runtime. This is only possible with CSS variables. This PR uses a CSS variable for input-border-thickness and checks the devices pixelRatio to either give it a 1.5px border or a 1px border.
Additionally the calculation for taking the border width into account was moved from $input-vertical-padding to the components themselves because sometime the border has to be removed twice (button) or only once (input).
QA
- Open demo
- Look at any input element
- Verify, that the correct border amount has been substracted from the padding - on both high (>1.99dppx) and low pixel density screens.
Check if PR is ready for release
If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:
- [x] PR should have one of the following labels to automatically categorise it in release notes:
Feature 🎁,Breaking Change 💣,Bug 🐛,Documentation 📝,Maintenance 🔨.
- [ ] Vanilla version in
package.jsonshould be updated relative to the most recent release, following semver convention:- if CSS class names are not changed it can be bugfix relesase (x.x.X)
- if CSS class names are changed/added/removed it should be minor version (x.X.0)
- see the wiki for more details
- [ ] Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.
Screenshots
Before
After
@bartaz I created a PR for the border calculations based on CSS variables as discussed in the other PR, just so we don't forget about this.
There is another issue I've noticed (it's why the tests are failing). Vanilla is not supposed to produce any CSS if you just import it. You need to explicitly include something to produce CSS.
By adding media query to settings we are breaking this rule, as there is CSS (media query definition of the border width value) in settings.
I'll have to think how best to avoid it. I guess we would need to define a default value of $input-border-width in settings, but the media query to change it would need to live elsewhere (somewhere in base styles), so it has to be explicitly included with Vanilla (but doesn't leak into CSS right away).
I reverted the variables back to the scss variable and renamed the css variable.
Hey @dgtlntv, did you have any thoughts or updates here? Going through and trying to push along some older PRs, so just curious what the status of this is.
@pastelcyborg As Bartek commented on in one of the threads there is this issue with my approach that there should not be CSS in the settings files. I don't really know how I'd go about fixing that so Bartek said he'd be looking into it.
So this seems to have been closed accidentally. We discussed it and I was wondering if @bartaz can find a better place for that media query?
I would change the value within the querry to 2px, and if this doesn't solve our problem, I suggest simply changing the default line thickness from 1.5 to 2px, which should require no queries.