nolanlawson / emoji-picker-element

A lightweight emoji picker for the modern web
https://nolanlawson.github.io/emoji-picker-element/
Apache License 2.0
1.47k stars 83 forks source link

fix: move layout logic from JS to CSS #427

Closed nolanlawson closed 2 months ago

nolanlawson commented 3 months ago

This is a simpler solution to the current system of ResizeObserver-based measurement and margin-setting.

Rather than trying to detect when the scrollbar disappears (e.g. during searching), and then setting a margin on the favorites panel to match it, we can just use scrollbar-gutter: stable to keep a little space there at all times for the scrollbar.

Screenshot from 2024-06-16 10-10-22

Screenshot from 2024-06-16 10-10-28

This doesn't work exactly like before, because there is actually space for the scrollbar while searching, but I think it's actually nicer not to have the content shifting around during search anyway. Plus, we can remove a ton of code with this.

This doesn't work in WebKit, but most of the time Safari uses an overlay scrollbar anyway. Plus, this can be thought of as a progressive enhancement.

github-actions[bot] commented 3 months ago

📊 Tachometer Benchmark Results

Summary

benchmark-total

  • emoji-picker-element-change-tab: unsure 🔍 -1% - +1% (-0.36ms - +0.18ms)
    this-change vs tip-of-tree
  • emoji-picker-element-database-interactions: faster ✔ 1% - 6% (0.53ms - 6.24ms)
    this-change vs tip-of-tree
  • emoji-picker-element-first-load: unsure 🔍 -5% - +2% (-2.11ms - +1.03ms)
    this-change vs tip-of-tree
  • emoji-picker-element-second-load: unsure 🔍 -8% - +3% (-2.89ms - +1.08ms)
    this-change vs tip-of-tree

Results

emoji-picker-element-change-tab
  • Browser: chrome-headless 126.0.6478.61
  • Sample size: 50
  • Built by: Benchmarks #444
  • Commit: b494659
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
33.28ms - 33.41ms-unsure 🔍
-1% - +1%
-0.36ms - +0.18ms
tip-of-tree
tip-of-tree
33.17ms - 33.70msunsure 🔍
-1% - +1%
-0.18ms - +0.36ms
-
emoji-picker-element-database-interactions
  • Browser: chrome-headless 126.0.6478.61
  • Sample size: 50
  • Built by: Benchmarks #444
  • Commit: b494659
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
99.88ms - 103.74ms-faster ✔
1% - 6%
0.53ms - 6.24ms
tip-of-tree
tip-of-tree
103.10ms - 107.30msslower ❌
0% - 6%
0.53ms - 6.24ms
-
emoji-picker-element-first-load
  • Browser: chrome-headless 126.0.6478.61
  • Sample size: 50
  • Built by: Benchmarks #444
  • Commit: b494659
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
44.53ms - 46.57ms-unsure 🔍
-5% - +2%
-2.11ms - +1.03ms
tip-of-tree
tip-of-tree
44.90ms - 47.28msunsure 🔍
-2% - +5%
-1.03ms - +2.11ms
-
emoji-picker-element-second-load
  • Browser: chrome-headless 126.0.6478.61
  • Sample size: 50
  • Built by: Benchmarks #444
  • Commit: b494659
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
32.88ms - 35.73ms-unsure 🔍
-8% - +3%
-2.89ms - +1.08ms
tip-of-tree
tip-of-tree
33.82ms - 36.59msunsure 🔍
-3% - +9%
-1.08ms - +2.89ms
-

tachometer-reporter-action v2 for Benchmarks

github-actions[bot] commented 3 months ago

Size Change: -133 B (-0.37%)

Total Size: 36.3 kB

Filename Size Change
./bundle.js 36.3 kB -133 B (-0.37%)

compressed-size-action

nolanlawson commented 3 months ago

Hm actually this doesn't quite work, because I forgot about isRtl and numColumns, which need to be calculated as well...

nolanlawson commented 3 months ago

isRtl is easy because I can use :dir(), but for numColumns there's no real way I can watch for changes to the computed styles... If --num-columns is set lazily then the favorites don't update. :thinking:

nolanlawson commented 3 months ago

Maybe it's fine; most people are going to set --num-columns initially anyway, not after the picker has already rendered.

nolanlawson commented 3 months ago

One minor issue is that the indicator animations will be a bit messed up for older versions of Safari which don't support :dir(). Detecting support for :dir() requires JS, though, and this only affects RTL users, and the animation is only a bit odd looking, so I think it's acceptable.

nolanlawson commented 3 months ago

Well if you're using Safari and have "always show scrollbars," then the alignment is off for the favorites bar, but I don't think it's a dealbreaker. It's a pretty rare setting anyway, I imagine.

Screenshot from 2024-06-16 15-58-55

nolanlawson commented 2 months ago

This might have been over-ambitious. I really want to use scrollbar-stable: gutter; maybe I should just focus on that.