Skip to content

Conversation

@emmadesilva
Copy link
Member

@emmadesilva emmadesilva commented Mar 24, 2024

Targets #1568

Since null is not allowed, we needed to coalesce to an empty string. This makes no sense from a practical usability perspective: What's the point of a link in the menu that you can't click because there is no label to click? It makes more sense to fall back to the link to make it usable, and give a nudge to the developer that they may want to change it into something more visually pleasing.

@codecov
Copy link

codecov bot commented Mar 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.93%. Comparing base (27fae01) to head (ea50e0f).

❗ Current head ea50e0f differs from pull request most recent head 8f2bd58. Consider uploading reports for the commit 8f2bd58 to get more accurate results

Additional details and impacted files
@@                       Coverage Diff                        @@
##             improved-navigation-internals    #1638   +/-   ##
================================================================
  Coverage                            99.93%   99.93%           
- Complexity                            1779     1780    +1     
================================================================
  Files                                  184      184           
  Lines                                 4802     4807    +5     
================================================================
+ Hits                                  4799     4804    +5     
  Misses                                   3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emmadesilva emmadesilva marked this pull request as ready for review March 24, 2024 16:52
@emmadesilva emmadesilva merged commit c33a6ec into improved-navigation-internals Mar 24, 2024
@emmadesilva emmadesilva deleted the create-better-fallbacks-when-not-supplying-a-navigation-item-label branch March 24, 2024 16:53
@emmadesilva emmadesilva added this to the v2 milestone Jul 9, 2024
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