Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: put annotation in title/aria-label #369

Merged
merged 2 commits into from Oct 8, 2023
Merged

fix: put annotation in title/aria-label #369

merged 2 commits into from Oct 8, 2023

Conversation

nolanlawson
Copy link
Owner

@nolanlawson nolanlawson commented Oct 8, 2023

Fixes #366

The title now comes from the annotation (if available - custom emoji may not have these). If not, it falls back to the shortcodes separated by a comma.

I also added the annotation to the aria-label because it seems like a nice enhancement – the annotation is often a better description than the shortcodes.

I don't see a reason to remove the shortcodes from the aria-label – it's just additional information, and the screenreader user can tab to the next emoji if they don't want to hear the shortcodes listed.

One downside of this is that there is no way for a sighted mouse user to figure out the shortcodes, but this is not vital to me since sighted mobile users were never able to see the title anyway.

@github-actions
Copy link

github-actions bot commented Oct 8, 2023

📊 Tachometer Benchmark Results

Summary

benchmark-total

  • emoji-picker-element-database-interactions: unsure 🔍 -2% - +6% (-2.38ms - +8.27ms)
    this-change vs tip-of-tree
  • emoji-picker-element-first-load: unsure 🔍 -9% - +6% (-5.00ms - +3.30ms)
    this-change vs tip-of-tree
  • emoji-picker-element-second-load: unsure 🔍 -9% - +3% (-5.49ms - +1.86ms)
    this-change vs tip-of-tree

Results

emoji-picker-element-database-interactions
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
132.36ms - 140.82ms-unsure 🔍
-2% - +6%
-2.38ms - +8.27ms
tip-of-tree
tip-of-tree
130.41ms - 136.88msunsure 🔍
-6% - +2%
-8.27ms - +2.38ms
-
emoji-picker-element-first-load
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
54.58ms - 60.25ms-unsure 🔍
-9% - +6%
-5.00ms - +3.30ms
tip-of-tree
tip-of-tree
55.24ms - 61.30msunsure 🔍
-6% - +9%
-3.30ms - +5.00ms
-
emoji-picker-element-second-load
VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
this-change
54.37ms - 59.63ms-unsure 🔍
-9% - +3%
-5.49ms - +1.86ms
tip-of-tree
tip-of-tree
56.24ms - 61.39msunsure 🔍
-3% - +10%
-1.86ms - +5.49ms
-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link

github-actions bot commented Oct 8, 2023

Size Change: +86 B (0%)

Total Size: 42.8 kB

Filename Size Change
./bundle.js 42.8 kB +86 B (0%)

compressed-size-action

@nolanlawson nolanlawson merged commit bd2004b into master Oct 8, 2023
3 checks passed
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.

Underscores in when hovering over emoji in the picker
1 participant