components icon indicating copy to clipboard operation
components copied to clipboard

feat: Add wrapTriggerText to popover

Open avinashbot opened this issue 1 year ago • 2 comments

Description

Added a wrapTriggerText property.

Follows the "negative" prop convention we essentially established for button and status indicator components. It has the name trigger in it to both follow the pattern with the other props (triggerType, triggerAriaLabel), and make it clear that this doesn't apply to the body itself. Setting this prop to false adds both text wrapping and truncation to the text trigger, and while I messed around with making it default, I've seen enough popovers with "internal" wrapping to realize this is a bad idea (plus it would make it completely unreadable/inaccessible for most cases outside a resizable table).

API changes

/**
 * Specifies if the text trigger content should wrap. If you set it to false, it prevents the text from
 * wrapping and truncates it with an ellipsis.
 */
wrapTriggerText?: boolean;

Copied almost verbatim from the status indicator (except I added the word "trigger").

Related links, issue #, if available: AWSUI-38391

How has this been tested?

Added permutation tests. Updated snapshots.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

avinashbot avatar Apr 30 '24 08:04 avinashbot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.37%. Comparing base (5efdd9b) to head (038df63).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2209    +/-   ##
========================================
  Coverage   95.37%   95.37%            
========================================
  Files         696      696            
  Lines       18584    18587     +3     
  Branches     6200     5887   -313     
========================================
+ Hits        17725    17728     +3     
- Misses        805      852    +47     
+ Partials       54        7    -47     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 30 '24 08:04 codecov[bot]

As discussed on the offline meeting, let's see if we can do wrapTriggerText="inherit" to let this component inherit the behavior from the parent (most likely, our table)

just-boris avatar Apr 30 '24 10:04 just-boris