fix(ui): topbar 🤝 accessibility reports #26
Conversation
src/browser/topbar.ts
Outdated
| const style = canvas.style | ||
| style.position = 'fixed' | ||
| style.top = style.left = style.right = style.margin = style.padding = 0 | ||
| style.zIndex = 100001 | ||
| style.top = style.left = style.right = style.margin = style.padding = '0' |
There was a problem hiding this comment.
Is it intentional? Is a string a better/proper way to code it? If yes - I'm 👍
Same q for the next line
There was a problem hiding this comment.
Is it intentional? Is a string a better/proper way to code it? If yes - I'm 👍
Yes, it is proper way according to HTMLCanvasElement type 😊 When I added const locally, the linter instantly became unhappy
src/browser/topbar.ts
Outdated
| canvas.style.opacity = `${Number(canvas.style.opacity) - 0.05}` | ||
| if (Number(canvas.style.opacity) <= 0.05) { |
There was a problem hiding this comment.
@kirillgroshkov I like this less than previous code but now it is correct type-wise. What do you think? 😬
There was a problem hiding this comment.
Frankly, I like how it was before :)
Previously this file was in the vendor folder, which I treat as "it's not our code, so we don't apply linters and rules for it, it just works as-is". Because of that - I feel more comfortable keeping things as-is, and making this PR a 1-liner - only adding the accessibility thing.
BTW, on accessibility. We don't currently use, and AFAIK don't plan to use topbar in production, because, for example, its UI is arbitrary/colorful and not on brand. It's only purpose is to aid in "developer mode" (similar to RedDot). Which would even be exempt from accessibility requirements. But I don't mind adding a line so accessibility audits don't scream.
There was a problem hiding this comment.
Previously this file was in the vendor folder, which I treat as "it's not our code, so we don't apply linters and rules for it, it just works as-is". Because of that - I feel more comfortable keeping things as-is, and making this PR a 1-liner - only adding the accessibility thing.
Makes sense. Updated the PR 👍
nikitalpopov
changed the title
nikitalpopov
force-pushed
the
DEV-19964
branch
from
a3257ed to
63e30d0
Compare
No description provided.