Skip to content

use SVG for boxing#620

Open
ysulyma wants to merge 7 commits intonumberscope:mainfrom
ysulyma:svg_box_icons
Open

use SVG for boxing#620
ysulyma wants to merge 7 commits intonumberscope:mainfrom
ysulyma:svg_box_icons

Conversation

@ysulyma
Copy link
Contributor

@ysulyma ysulyma commented Feb 2, 2026

Unicode boxing characters ( and ) render inconsistently between browsers and have mismatched widths. I changed them to SVG components.

Before

Firefox (misaligned)

image

Chrome/Safari (better alignment, mismatched widths)

image

After

all three browsers:
image

@gwhitney
Copy link
Collaborator

gwhitney commented Feb 2, 2026

Thanks for the visual improvement (that we hadn't noticed as the Unicode comes out great on our computers...)!
I am guessing you have not installed our commit actions, which is entirely up to you. However, we do nevertheless want all PRs to pass the tests in .husky/pre-commit, namely:

npm run lint:check
npm run test:unit
npm run test:e2e

In particular, some of your changed files are not conformant to our lint settings (you can try to auto-fix with npm run lint if you like), and also your build is failing with TypeScript declaration issues; you can reproduce this latter issue with npm run build. Thanks in advance for updating your PR when you have a chance.

@gwhitney
Copy link
Collaborator

gwhitney commented Feb 2, 2026

In particular for the build issues, it might be enough to add lang="ts" to the <script setup> tag, see here.

@gwhitney
Copy link
Collaborator

gwhitney commented Feb 2, 2026

What's the rationale for setting a default height and width of 32 for the boxes and then using them at height 28 and width 24? Since there's only a unique use of each at the moment, and for the moderate foreseeable future, isn't it cleaner to have the defaults be the actual used height and width for now, and then not have to reiterate them in the uses?

@gwhitney
Copy link
Collaborator

gwhitney commented Feb 2, 2026

Thanks for updating! I see you only changed the default values in one of the two new BoxXxxx components, so they don't match any more. I would likely have just made them uniform myself and merged, except I noticed two other things:

  1. Now for me in Firefox on (OpenSUSE Tumbleweed) Linux, the "Corner" is noticeably lighter weight than the "Join". (See attached screenshot.)

  2. The help dropdown is now quite likely to disappear as I move the mouse down from the Help navbar link (that makes it appear) to one of the items in the dropdown, which is quite frustrating. If I really zip the mouse fast, I can get the dropdown to stay up and manage to select one of the items, but with my normal mouse motion it disappears more than half the time before I can manage to select one of the dropdown items. I compared this situation to main, in which by sneaking the mouse down slowly I actually could get the dropdown to disappear, which I had never noticed before; in my usual mouse motion, the dropdown stays up at least 90% of the time.

Any thoughts/fixes on these two things? My guess on the second is that there is a very thin horizontal space in which neither the navbar link or the dropdown is considered hovered, and when the browser detects that, it closes the dropdown. Presuming so, that's a bug in main that we'd love fixed if you have any thoughts on doing so. And then my guess is that the subtle formatting changes in this PR slightly increased the height of this "no-person's-land", making the browser much more likely to close the dropdown. Again, presuming so, we definitely don't want to go in that direction since, at least in my environment as described above, it makes the Help menu very frustrating to use.

@gwhitney
Copy link
Collaborator

gwhitney commented Feb 2, 2026

Oops, here's the screenshot.
Screenshot_2026-02-02_21-53-15

@ysulyma ysulyma mentioned this pull request Feb 3, 2026
@ysulyma
Copy link
Contributor Author

ysulyma commented Feb 3, 2026

@gwhitney

  1. Does this persist after fixing the defaults on the other one?

  2. Inadvertently introduced by fix alignment of navbar links #606, apologies! I fixed in fix help menu gap #625

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.

2 participants