fix(item-sliding, segment-button): read props after frameworks assign them

core/src/components/item-sliding/item-sliding.tsx

@@ -1,7 +1,7 @@
 import type { ComponentInterface, EventEmitter } from '@stencil/core';
 import { Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
 import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content';
-import { isEndSide } from '@utils/helpers';
+import { componentOnReady, isEndSide } from '@utils/helpers';
 import { printIonWarning } from '@utils/logging';
 import { watchForOptions } from '@utils/watch-options';
 
@@ -245,24 +245,21 @@ export class ItemSliding implements ComponentInterface {
   }
 
   private async updateOptions() {
-    const options = this.el.querySelectorAll('ion-item-options');
+    const options = Array.from(this.el.querySelectorAll('ion-item-options'));
+
+    /**
+     * Frameworks that assign element props after inserting the element haven't set
+     * `side` while `connectedCallback` runs, so reading it any earlier reports every
+     * option as `end`.
+     */
+    await Promise.all(options.map((option) => new Promise((resolve) => componentOnReady(option, resolve))));
 
     let sides = 0;
 
     // Reset left and right options in case they were removed
     this.leftOptions = this.rightOptions = undefined;
 
-    for (let i = 0; i < options.length; i++) {
-      const item = options.item(i);
-
-      /**
-       * We cannot use the componentOnReady helper
-       * util here since we need to wait for all of these items
-       * to be ready before we set `this.sides` and `this.optsDirty`.
-       */
-      // eslint-disable-next-line custom-rules/no-component-on-ready-method
-      const option = (item as any).componentOnReady !== undefined ? await item.componentOnReady() : item;
-
+    for (const option of options) {
       const side = isEndSide(option.side ?? option.getAttribute('side')) ? 'end' : 'start';
 
       if (side === 'start') {

core/src/components/item-sliding/test/basic/item-sliding.e2e.ts

@@ -1,4 +1,5 @@
 import { expect } from '@playwright/test';
+import type { E2EPage } from '@utils/test/playwright';
 import { configs, dragElementBy, test } from '@utils/test/playwright';
 
 /**
@@ -190,3 +191,76 @@ configs().forEach(({ title, screenshot, config }) => {
     });
   });
 });
+
+/**
+ * ion-item-sliding reads `side` off each ion-item-options to decide which way the item
+ * can open. Frameworks that assign element props after inserting the element haven't set
+ * it while `connectedCallback` runs.
+ *
+ * The shared harness page is used because it loads the custom elements build, which is
+ * where that ordering applies.
+ *
+ * This behavior does not vary across modes or directions.
+ */
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
+  test.describe(title('item-sliding: basic'), () => {
+    const openStartOptions = async (page: E2EPage, lateProps: boolean) => {
+      await page.goto('/src/utils/test/late-props', config);
+      await page.waitForFunction(() => (window as any).harnessReady === true);
+
+      await page.evaluate(
+        (late: boolean) =>
+          (window as any).mountLateProps(
+            ['ion-content', 'ion-list', 'ion-item', 'ion-item-sliding', 'ion-item-options', 'ion-item-option'],
+            {
+              tag: 'ion-content',
+              children: [
+                {
+                  tag: 'ion-list',
+                  children: [
+                    {
+                      tag: 'ion-item-sliding',
+                      children: [
+                        { tag: 'ion-item', children: [{ tag: 'p', children: ['No label'] }] },
+                        {
+                          // Passing `side` as a prop lets `lateProps` control when it can be read.
+                          tag: 'ion-item-options',
+                          props: { side: 'start' },
+                          children: [{ tag: 'ion-item-option', children: ['Favorite'] }],
+                        },
+                      ],
+                    },
+                  ],
+                },
+              ],
+            },
+            late
+          ),
+        lateProps
+      );
+      await page.waitForChanges();
+
+      const slidingItem = page.locator('ion-item-sliding');
+
+      // A positive drag pulls the item to the right, revealing the start options.
+      await dragElementBy(slidingItem, page, 150);
+      await page.waitForChanges();
+
+      await expect(slidingItem).toHaveClass(/item-sliding-active-options-start/);
+      await expect(page.locator('ion-item-options')).toBeVisible();
+    };
+
+    test('should open the start options when side is assigned before connecting', async ({ page }) => {
+      await openStartOptions(page, false);
+    });
+
+    test('should open the start options when side is assigned after connecting', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/31388',
+      });
+
+      await openStartOptions(page, true);
+    });
+  });
+});

core/src/components/segment-button/segment-button.tsx

@@ -73,14 +73,6 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
       addEventListener(segmentEl, 'ionSelect', this.updateState);
       addEventListener(segmentEl, 'ionStyle', this.updateStyle);
     }
-
-    // Prevent buttons from being disabled when associated with segment content
-    if (this.contentId && this.disabled) {
-      printIonWarning(
-        `[ion-segment-button] - Segment buttons cannot be disabled when associated with an <ion-segment-content>.`
-      );
-      this.disabled = false;
-    }
   }
 
   disconnectedCallback() {
@@ -100,6 +92,18 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
     // Return if there is no contentId defined
     if (!this.contentId) return;
 
+    /**
+     * Checked here rather than in `connectedCallback` so frameworks that assign element
+     * props after inserting the element have set `disabled` by now. A disabled ion-segment
+     * pushes that onto its buttons too, which this guard should not undo.
+     */
+    if (this.disabled && this.segmentEl?.disabled !== true) {
+      printIonWarning(
+        `[ion-segment-button] - Segment buttons cannot be disabled when associated with an <ion-segment-content>.`
+      );
+      this.disabled = false;
+    }
+
     // Attempt to find the Segment Content by its contentId
     const segmentContent = document.getElementById(this.contentId) as HTMLIonSegmentContentElement | null;
 

core/src/components/segment-view/test/disabled/segment-view.e2e.ts

@@ -47,3 +47,113 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
     });
   });
 });
+
+/**
+ * Frameworks that assign element props after inserting the element have set neither
+ * `contentId` nor `disabled` while `connectedCallback` runs, so the check that keeps a
+ * button enabled has to happen later.
+ *
+ * The shared harness page is used because it loads the custom elements build, which is
+ * where that ordering applies.
+ *
+ * This behavior does not vary across modes or directions.
+ */
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
+  test.describe(title('segment-view: disabled'), () => {
+    [false, true].forEach((lateProps) => {
+      const when = lateProps ? 'after connecting' : 'before connecting';
+
+      test(`should only re-enable the developer-disabled button when props are assigned ${when}`, async ({ page }) => {
+        const warnings: string[] = [];
+
+        page.on('console', (msg) => {
+          if (msg.type() === 'warning') {
+            warnings.push(msg.text());
+          }
+        });
+
+        await page.goto('/src/utils/test/late-props', config);
+        await page.waitForFunction(() => (window as any).harnessReady === true);
+
+        await page.evaluate(
+          (late: boolean) =>
+            (window as any).mountLateProps(
+              ['ion-segment', 'ion-segment-button', 'ion-segment-view', 'ion-segment-content', 'ion-label'],
+              {
+                tag: 'div',
+                children: [
+                  {
+                    // The developer disabled the second button, which has to be forced back on.
+                    tag: 'ion-segment',
+                    props: { value: 'first' },
+                    children: [
+                      {
+                        tag: 'ion-segment-button',
+                        props: { value: 'first', contentId: 'first-content' },
+                        children: [{ tag: 'ion-label', children: ['First'] }],
+                      },
+                      {
+                        tag: 'ion-segment-button',
+                        props: { value: 'second', contentId: 'second-content', disabled: true },
+                        children: [{ tag: 'ion-label', children: ['Second'] }],
+                      },
+                    ],
+                  },
+                  {
+                    tag: 'ion-segment-view',
+                    children: [
+                      { tag: 'ion-segment-content', attrs: { id: 'first-content' }, children: ['First'] },
+                      { tag: 'ion-segment-content', attrs: { id: 'second-content' }, children: ['Second'] },
+                    ],
+                  },
+                  {
+                    // This whole segment is disabled, so its buttons stay off.
+                    tag: 'ion-segment',
+                    props: { value: 'third', disabled: true },
+                    children: [
+                      {
+                        tag: 'ion-segment-button',
+                        props: { value: 'third', contentId: 'third-content' },
+                        children: [{ tag: 'ion-label', children: ['Third'] }],
+                      },
+                      {
+                        tag: 'ion-segment-button',
+                        props: { value: 'fourth', contentId: 'fourth-content' },
+                        children: [{ tag: 'ion-label', children: ['Fourth'] }],
+                      },
+                    ],
+                  },
+                  {
+                    tag: 'ion-segment-view',
+                    children: [
+                      { tag: 'ion-segment-content', attrs: { id: 'third-content' }, children: ['Third'] },
+                      { tag: 'ion-segment-content', attrs: { id: 'fourth-content' }, children: ['Fourth'] },
+                    ],
+                  },
+                ],
+              },
+              late
+            ),
+          lateProps
+        );
+        await page.waitForChanges();
+
+        const disabled = await page
+          .locator('ion-segment')
+          .evaluateAll((segments: HTMLIonSegmentElement[]) =>
+            segments.map((segment) =>
+              Array.from(segment.querySelectorAll('ion-segment-button')).map((button) => button.disabled)
+            )
+          );
+
+        expect(disabled).toEqual([
+          [false, false],
+          [true, true],
+        ]);
+        expect(warnings.join('\n')).toContain(
+          '[ion-segment-button] - Segment buttons cannot be disabled when associated with an <ion-segment-content>.'
+        );
+      });
+    });
+  });
+});

core/src/utils/test/late-props/index.html

@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Late Props</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0" />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+
+    <style>
+      /* Fill the app so a mounted ion-content has a sized containing block. */
+      #root {
+        position: absolute;
+        inset: 0;
+      }
+    </style>
+    <script type="module">
+      /**
+       * Imported by absolute path because this page is served without a trailing slash,
+       * so a relative specifier resolves against the parent directory.
+       */
+      import { defineTags, initializeIonic, mount } from '/src/utils/test/late-props/late-props.js';
+
+      initializeIonic();
+
+      await defineTags(['ion-app']);
+
+      /**
+       * Mounts `spec` into the page, defining whatever tags the caller needs first.
+       * When `lateProps` is true the props are assigned after the tree is connected.
+       */
+      window.mountLateProps = async (tags, spec, lateProps) => {
+        await defineTags(tags);
+
+        mount(document.getElementById('root'), spec, lateProps);
+      };
+
+      window.harnessReady = true;
+    </script>
+  </head>
+
+  <body>
+    <ion-app>
+      <div id="root"></div>
+    </ion-app>
+  </body>
+</html>

core/src/utils/test/late-props/late-props.js

@@ -0,0 +1,69 @@
+/**
+ * Test helpers for the custom elements build, where `connectedCallback` runs
+ * synchronously as the element is inserted. Frameworks that assign element props after
+ * inserting the element leave a window where a component can't read its own props or
+ * its children's, and it still has to work.
+ */
+
+import { initialize } from '/components/index.js';
+
+/**
+ * Initializes Ionic in the mode the test asked for.
+ */
+export const initializeIonic = () => {
+  initialize({ mode: new URLSearchParams(location.search).get('ionic:mode') ?? 'ios' });
+};
+
+/**
+ * Defines the given tags. Safe to call again for tags that are already defined, so each
+ * test can ask for whatever it needs.
+ */
+export const defineTags = async (tags) => {
+  await Promise.all(
+    tags.map(async (tag) => {
+      const mod = await import(`/components/${tag}.js`);
+      mod.defineCustomElement();
+    })
+  );
+};
+
+/**
+ * Builds the tree described by `spec` and appends it to `root`. A spec node is
+ * `{ tag, props, attrs, children }`, where `children` holds specs or strings. The
+ * `attrs` are always set before the element connects, and the `props` are set before
+ * connecting when `lateProps` is false, or after the whole tree connects when it is true.
+ */
+export const mount = (root, spec, lateProps) => {
+  const pending = [];
+
+  const build = (node) => {
+    if (typeof node === 'string') {
+      return document.createTextNode(node);
+    }
+
+    const el = document.createElement(node.tag);
+
+    if (node.attrs) {
+      Object.entries(node.attrs).forEach(([key, value]) => el.setAttribute(key, String(value)));
+    }
+
+    if (node.props) {
+      if (lateProps) {
+        pending.push([el, node.props]);
+      } else {
+        Object.assign(el, node.props);
+      }
+    }
+
+    (node.children || []).forEach((child) => el.appendChild(build(child)));
+
+    return el;
+  };
+
+  const tree = build(spec);
+
+  root.appendChild(tree);
+
+  // Descendants before ancestors, matching the order framework effects run in.
+  pending.reverse().forEach(([el, props]) => Object.assign(el, props));
+};