Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
285 stars 76 forks source link

[Action Bar] Cannot set `text-enabled` on slotted Action when `expand-disabled` is set #8886

Open macandcheese opened 7 months ago

macandcheese commented 7 months ago

Check existing issues

Actual Behavior

When expand-disabled is set on a containing Action Bar, I cannot set text-enabled on a slotted Action.

Additionally, when setting overflow-actions-disabled, and there are actions-end slotted actions, they are not presented horizontally. Maybe this is a pattern thing and they should not be expected within a group there.

Expected Behavior

I'd expect to be able to individually set text-enabled on a slotted Action when expand-disabled is set on a parent. The use case for this is primarily for layout="horizontal", where one or a few of the slotted Action are desired to have text-enabled.

Reproduction Sample

https://codepen.io/mac_and_cheese/pen/yLrNmXa?editors=1000

Reproduction Steps

  1. Open Codepen
  2. Notice the text-enabled properties set on slotted Actions are not respected
  3. Notice the overflow-actions-disabled example "stacks" the slotted actions-end when they are in a group

Reproduction Version

2.6.0

Relevant Info

From @driskull "we just need to update the toggleChildActionTextfunction to consider expandDisabled"

Regression?

No response

Priority impact

p4 - not time sensitive

Impact

No response

Calcite package

Esri team

Calcite (design)

driskull commented 3 months ago

@macandcheese it looks like when a component is connected or when a new action is slotted, we are modifying that action to be text-enabled or not depending on the state of the parent component.

I think we should change this but it would be a fix that users would need to be aware of and maybe modify their code to handle. For example, if a user adds an action in a calcite-action-bar that has expanded=true then they should make sure that the action has text-enabled.

The change would be to only modify text-enabled on child actions when the expanded property is changed. Not when a component is loaded, connected, or during a mutation.

cc @jcfranco for if we want to do this during a breaking change release.

driskull commented 3 months ago

@geospatialem @DitwanP I think we should move this one to the breaking change release since it could require users to make changes to their code.

jcfranco commented 3 months ago

Sounds good. Moving to the November milestone as a precaution. Also labeled as breaking to remind us to add additional context for this change.