clients
clients copied to clipboard
[PS-1512] Desktop: Fix inconsistent line height in cipher box footer
Closes #3559
Type of change
- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
In the box footer for a cipher in desktop - which contains "Updated", "Password updated", "Password history" - the "Password history" line is slightly taller than the other two. Line height is 17.1429px
on all of them, but the last line has a button with a height of 20px
that pushed the overall line height to 20px
. Nothing dramatic, but to a keen design eye, it just visibly looks slightly "off".
This PR forces the line height for buttons to match the parent container. For consistency, applying the same style also to .box-footer a
elements, even though currently (e.g. in the equivalent browser popup) there's no problem just now. This just makes sure that if in future things change, there won't be a visual regression.
Code changes
-
apps/browser/src/popup/scss/misc.scss and apps/desktop/src/scss/misc.scss: add specific style rule to set explicit
line-height: 1
Screenshots
Before - note the slightly uneven line spacing (with the last line being slightly too high)
After - with the line heights evened out
(note that the difference is admittedly very subtle...change from 20px
to 17.1429px
height on the last line)
Before you submit
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team
Thank you for your contribution! We've added this to our internal Community PR board for review. ID: PS-1512
@patrickhlauke Could we instead increase the line height of all of these to 20px?
Also, this might not be what you were working on with this but I noticed that the that launches the password history modal/popup doesn't seem to be keyboard focusable currently, do you have any thoughts on how we could improve that?
![Screen Shot 2022-09-19 at 12 10 42 PM](https://user-images.githubusercontent.com/95641831/191075914-532ff939-d8b2-4afe-be4d-3cf4f91101a8.png)
@patrickhlauke Could we instead increase the line height of all of these to 20px?
That's a larger design decision that I'll leave up to you/the team. If/when you do that, the line-height: 1
here will adapt the buttons to whatever you set.
Also, this might not be what you were working on with this but I noticed that the that launches the password history modal/popup doesn't seem to be keyboard focusable currently, do you have any thoughts on how we could improve that?
works for me. it's a button that takes focus (that <span>
you highlighted is inside the button)
@patrickhlauke Could we instead increase the line height of all of these to 20px?
That's a larger design decision that I'll leave up to you/the team. If/when you do that, the
line-height: 1
here will adapt the buttons to whatever you set.Also, this might not be what you were working on with this but I noticed that the that launches the password history modal/popup doesn't seem to be keyboard focusable currently, do you have any thoughts on how we could improve that?
works for me. it's a button that takes focus (that
<span>
you highlighted is inside the button)
To clarify, if the row height for each line of text could be the equivalent of 20px so the text is more readable that would be awesome. Thanks!
To clarify, if the row height for each line of text could be the equivalent of 20px so the text is more readable that would be awesome. Thanks!
again, i think that's a larger design decision that should be made globally throughout the app / by design team. i'll leave that out of this PR (but when/if that's done, the change here in the PR will adapt to the new line height)
but when/if that's done, the change here in the PR will adapt to the new line height)
Gotcha, I understand you now.
my design-obsessed eyes thank you (one of those "once i've seen it, I can't unsee it" things) :)