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 ListItem Disabled Behavior #1000

Merged
merged 2 commits into from
Nov 14, 2020
Merged

Fix ListItem Disabled Behavior #1000

merged 2 commits into from
Nov 14, 2020

Conversation

mlaursen
Copy link
Owner

@mlaursen mlaursen commented Nov 14, 2020

This PR includes:

  • Preventing the ListItem from being focusable by default when disabled. This better mimics the button behavior that it inherits, but can still be set to 0 or another tabIndex if needed.
  • Correctly set the disabled color to ListItem secondary text
  • Adding a new prop to the ListItem that allows the disabled state to be applied with opacity instead of colors. This allows for the addons to also gain a disabled state
  • introduce a new $rmd-list-item-disabled-opacity: 0.5! default; value for the disabledOpacity prop

Closes #997

Visual Examples

Light Theme

Default ListItem

ListItem Example

Disabled ListItem

ListItem Example

Disabled Opacity ListItem

ListItem Example

Default ListItem with Addons

ListItem Example

Disabled ListItem with Addons

ListItem Example

Disabled Opacity ListItem with Addons

ListItem Example

Dark Theme

Default ListItem

ListItem Example

Disabled ListItem

ListItem Example

Disabled Opacity ListItem

ListItem Example

Default ListItem with Addons

ListItem Example

Disabled ListItem with Addons

ListItem Example

Disabled Opacity ListItem with Addons

ListItem Example

This better mimics the button behavior that it inherits, but can still
be set to 0 or another tabIndex if needed.

fix #997
The new `disabledOpacity` prop will allow for the full ListItem to gain
an opacity value while disabled instead of only setting the color
values. This allows the addons to also be dimmed.

fix #997
@github-actions github-actions bot added the list label Nov 14, 2020
@codecov-io
Copy link

Codecov Report

Merging #1000 (a40b6b3) into main (0bc495d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1000   +/-   ##
=======================================
  Coverage   88.89%   88.90%           
=======================================
  Files         248      248           
  Lines        6980     6983    +3     
  Branches     1808     1812    +4     
=======================================
+ Hits         6205     6208    +3     
  Misses        698      698           
  Partials       77       77           
Impacted Files Coverage Δ
packages/list/src/getListItemHeight.ts 100.00% <ø> (ø)
packages/list/src/ListItem.tsx 100.00% <100.00%> (ø)
packages/list/src/SimpleListItem.tsx 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0bc495d...a40b6b3. Read the comment docs.

@mlaursen mlaursen merged commit a40b6b3 into main Nov 14, 2020
@mlaursen mlaursen deleted the bugfix/list-item-color branch November 14, 2020 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling a ListItem does not grey out secondaryText color
2 participants