fix(combobox, stepper, table, tip-manager): respect user hidden attribute #10515

josercarcamo · 2024-10-09T19:48:57Z

Related Issue: #8623

Summary

Made items with the hidden attribute added by the user stay hidden.

jcfranco

Solid start, @josercarcamo! Can you look at introducing a pattern for components that need to control visibility of their children? Namely to:

define a property to use instead of hidden
add utilities to help determine visibility of these items (e.g., checks for hidden and the property defined in 1)

You've got most of this in place already. 🎉

Here are some notes:

let's avoid user in the internal hidden prop name
we can omit adding stories since hidden is a global attribute and we don't test these
you can skip all tip-related components since these are deprecated and will be removed in v4.

jcfranco · 2024-10-21T22:27:32Z

packages/calcite-components/src/components/stepper-item/stepper-item.tsx

@@ -217,6 +217,9 @@ export class StepperItem
  }

  async componentWillLoad(): Promise<void> {
+    if (this.el.hidden) {


FIXME: Returning early will prevent the component from properly initializing.

jcfranco · 2024-10-21T22:29:00Z

packages/calcite-components/src/components/stepper-item/stepper-item.tsx

@@ -253,7 +258,7 @@ export class StepperItem
          <div class={CSS.container}>
            {this.complete && (
              <span aria-live="polite" class={CSS.visuallyHidden}>
-                {this.messages.complete}
+                {this.messages?.complete}


Adding an optional chain operator shouldn't be necessary. This is caused by skipping initialization logic in component lifecycle hooks.

jcfranco · 2024-10-21T22:30:58Z

packages/calcite-components/src/components/stepper-item/stepper-item.tsx

@@ -386,6 +391,7 @@ export class StepperItem
  private handleItemClick = (event: MouseEvent): void => {
    if (
      this.disabled ||
+      event.composedPath().some((el) => (el as HTMLElement).hidden) ||


Was this added for a particular use case? hidden in most cases would not display the element and thus not be clickable. Applies to other hidden-related logic in this PR.

This might affect test changes.

Yes, this was added for the test case "should emit calciteStepperChange/calciteStepperItemChange on user interaction" where it directly queries for an element and clicks on it, even when it is disabled and, with this change, hidden.

jcfranco · 2024-10-21T22:32:54Z

packages/calcite-components/src/components/stepper/stepper.e2e.ts

-      expect(await step2Content.isVisible()).toBe(false);
-      expect(await step3Content.isVisible()).toBe(false);
+    it("navigates disabled and hidden items correctly with nextStep and prevStep methods", async () => {
+      for (const n of ["disabled", "hidden"]) {


FIXME: avoid unnecessary/non-standard abbreviations.

jcfranco · 2024-10-21T22:38:41Z

packages/calcite-components/src/components/table-row/table-row.tsx

@@ -119,6 +122,10 @@ export class TableRow implements InteractiveComponent, LocalizedComponent {
  //
  //--------------------------------------------------------------------------

+  connectedCallback(): void {
+    this.userHidden = this.el.hidden;


FIXME: This property is prone to getting out of sync since there are no configured watchers. If we introduce a new, internal prop for hiding components (if needed by their spec), this will be simplified.

jcfranco · 2024-10-21T22:40:28Z

packages/calcite-components/src/components/tip-manager/tip-manager.tsx

  }

  async componentWillLoad(): Promise<void> {
    await setUpMessages(this);
+    this.setUpTips();


FIXME: this needs to be restored, so they are called again whenever the component is moved or added to the DOM.

jcfranco · 2024-10-23T17:51:15Z

packages/calcite-components/src/components/stepper/stepper.e2e.ts

-      expect(await step2Content.isVisible()).toBe(false);
-      expect(await step3Content.isVisible()).toBe(false);
+    it("navigates disabled and hidden items correctly with nextStep and prevStep methods", async () => {
+      for (const n of ["disabled", "hidden"]) {


FIXME: Since testing both hidden and disabled are related here, can you update the test by adding a hidden item and updating assertions instead of running another test? Applies to all tests in this and other E2E suites.

This would render the above abbreviation FIXME obsolete.

jcfranco · 2024-10-23T17:53:50Z

packages/calcite-components/src/components/stepper/stepper.e2e.ts

@@ -870,6 +884,50 @@ describe("calcite-stepper", () => {
      expect(itemChangeSpy).toHaveReceivedEventTimes(2);
    });

+    it("should emit calciteStepperItemChange on user interaction when item is hidden", async () => {


Can you merge this with the should emit calciteStepperItemChange on user interaction test (i.e., tests would include both hidden and disabled items)?

github-actions · 2024-11-01T02:03:56Z

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

github-actions · 2024-12-03T02:04:55Z

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

josercarcamo · 2024-12-16T16:59:53Z

Superseded by #10983

**Related Issue:** #8623 ## Summary Made items with the hide-item attribute added by the user stay hidden. This PR supersedes #10515 ## Wiki page https://github.com/Esri/calcite-design-system/wiki/hiddenItem-Property

josercarcamo added 6 commits September 26, 2024 22:50

compeonts

f4037f7

Merge branch 'dev' of github.com:Esri/calcite-design-system into dev

801a95c

fix(combobox): respect hidden attribute in components

6452634

Corrected how to respect hidden on combobox

4912965

Made stepper respect hidden

383a65a

Made table and tips respect hidden

6a27c1c

github-actions bot assigned josercarcamo Oct 9, 2024

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Oct 9, 2024

josercarcamo marked this pull request as draft October 9, 2024 19:49

josercarcamo and others added 11 commits October 9, 2024 14:41

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

e0cbeef

Updated/added tests to check for hidden attribute

0c39f31

Added hidden tests for combobox

39196ed

Fixed combo box tests for hidden

7eb24aa

Updated and added tests for table with hidden attribute

5fc7f1b

Added test for tip-manager with the hidden attribute

18e9723

Added test for combobox hidden attribute

738d4e3

Added hidden attribute to stepper

72c52f9

Added test for table with hidden attribute

185fad3

Added test for tip-manager with hidden attribute

a97c6c1

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

d78d198

josercarcamo added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 10, 2024

josercarcamo and others added 9 commits October 10, 2024 15:19

Merge branch 'dev' of github.com:Esri/calcite-design-system into dev

df0fc4d

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

5869d95

Merge branch 'dev' of github.com:Esri/calcite-design-system into dev

16ba7f8

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

2c2e1c6

Blah

fd28e9a

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

cac8214

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

1c96552

Fixed failing comobobox tests

e03ad1c

Fixed failing tests

4f68108

josercarcamo and others added 3 commits October 18, 2024 16:35

Fixed failing tests

3033b60

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

d91adc5

Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribute

cdaf9c3

josercarcamo added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 21, 2024

josercarcamo marked this pull request as ready for review October 23, 2024 17:36

jcfranco requested changes Oct 23, 2024

View reviewed changes

github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Nov 1, 2024

Merged dev

cd9e5bc

josercarcamo requested a review from benelan as a code owner November 13, 2024 18:41

josercarcamo added 9 commits November 13, 2024 15:07

Refactored code per code review

b87c2c4

Removed all changes to tip and tip-manager

089ce28

Removed tip changes per code review

aa5059e

Replaced 'userHidden' with 'hideItem' property

fbdf30e

Removed all stories per code review

7957b07

Fixed test cases for hide-item

55f7df5

Combined test cases for disabled and hidden

52208bc

Repaced 'hidden' with 'hide-item'

7ace7a6

Removed unnecessary if statement

d94d8db

github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label Nov 16, 2024

josercarcamo force-pushed the josercarcamo/8623-respect-user-hidden-attribute branch from 827a0bc to d94d8db Compare November 25, 2024 21:55

github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Dec 3, 2024

josercarcamo mentioned this pull request Dec 5, 2024

fix(combobox, stepper, table): respect user hidden attribute #10983

Merged

josercarcamo closed this Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(combobox, stepper, table, tip-manager): respect user hidden attribute #10515

fix(combobox, stepper, table, tip-manager): respect user hidden attribute #10515

josercarcamo commented Oct 9, 2024

jcfranco left a comment •

edited

Loading

jcfranco Oct 21, 2024

jcfranco Oct 21, 2024

jcfranco Oct 21, 2024

josercarcamo Nov 15, 2024

jcfranco Oct 21, 2024

jcfranco Oct 21, 2024

jcfranco Oct 21, 2024

jcfranco Oct 23, 2024

jcfranco Oct 23, 2024

github-actions bot commented Nov 1, 2024

github-actions bot commented Dec 3, 2024

josercarcamo commented Dec 16, 2024

fix(combobox, stepper, table, tip-manager): respect user hidden attribute #10515

fix(combobox, stepper, table, tip-manager): respect user hidden attribute #10515

Conversation

josercarcamo commented Oct 9, 2024

Summary

jcfranco left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 1, 2024

github-actions bot commented Dec 3, 2024

josercarcamo commented Dec 16, 2024

jcfranco left a comment •

edited

Loading