INSTUI-4672 Add the (almost complete) regression test suite, fix small a11y issues, SSR errors by matyasf · Pull Request #2105 · instructure/instructure-ui

matyasf · 2025-08-14T14:34:51Z

These tests are mostly AI generated based on our README examples.

Also fix an SSR error in Calendar: inThe weekday header IDs were getting out of sync because they were not deterministically generated
Also fix an a11y issue in Drilldown (see comments)
Also fix an a11y issue in Options (see comments)

Note: The large rendering delays are needed because there is a large layout shift in some components that use FormFieldLayout when the hydration happens because we calculate the grid placement in styles.ts, so the barebones DOM is a half-complete grid.

To test:

Launch the regression test app, check the examples. For now we're not looking for 100% coverage on all visual features, just the happy paths.
Check the changes in Chromatic

github-actions · 2025-08-14T14:40:20Z

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-08-29 08:35 UTC

matyasf · 2025-08-15T10:26:43Z

packages/ui-drilldown/src/Drilldown/index.tsx

      optionProps.renderAfterLabel = <IconArrowOpenEndSolid />
      optionProps['aria-haspopup'] = true
-      optionProps.role = customRole || 'button'
+      optionProps.role = customRole || 'menuitem'


Drilldown has role=menu, which should not have role=button children. We had a similar issue in the past: #1831 Also see the W3 spec: https://w3c.github.io/aria/#menu . Also this triggers a critical axe check failure (on our docs page too)

matyasf · 2025-08-15T10:27:59Z

regression-test/cypress/e2e/spec.cy.ts

  // Add a small wait if your application might log errors asynchronously
  cy.wait(100).then(() => {
-    expect(windowErrorSpy).to.not.be.called
+    expect(windowErrorSpy).to.have.callCount(0)


This makes it to output the actual error message to the console

matyasf · 2025-08-18T13:16:14Z

regression-test/src/app/alert/page.tsx

@@ -0,0 +1,62 @@
+/*


Note: All the component test pages were generated by AI using our READMEs as a basis. I dont think the code of these needs more than a very superficial checking, whats important how they look in Chromatic

matyasf · 2025-08-18T13:20:06Z

packages/ui-options/src/Options/Item/styles.ts

+    disabled: { cursor: 'not-allowed', opacity: 0.68 },
    'highlighted-disabled': {
      background: componentTheme.highlightedBackground,
      color: componentTheme.highlightedLabelColor,
-      cursor: 'not-allowed',
-      opacity: 0.5
+      cursor: 'not-allowed'


These needed the opacity to be increased, otherwise they would fail color contract a11y tests. AFAIK we are not using this directly anywhere, so it should be OK

matyasf · 2025-08-18T13:25:58Z

packages/ui-calendar/src/Calendar/index.tsx

+    this._weekdayHeaderIds = (
+      this.props.renderWeekdayLabels || this.defaultWeekdays
+    ).reduce((ids: Record<number, string>, _label, i) => {
+      return { ...ids, [i]: this.props.deterministicId!('weekday-header') }
+    }, {})


SSR bugfix due to ID not being deterministic

matyasf · 2025-08-18T13:27:31Z

packages/ui-heading/src/Heading/README.md

+  <Heading aiVariant="stacked" level="h2">Nutrition Facts</Heading>
+  <Heading aiVariant="horizontal" level="h3">Nutrition Facts</Heading>
+  <Heading aiVariant="iconOnly" level="h4">Nutrition Facts</Heading>


variant should be specified according to our own docs

You mean level instead of variant I guess.

matyasf · 2025-08-18T14:44:25Z

package.json

  },
  "engines": {
-    "node": ">=18",
+    "node": ">=22",


I've updated this to be in sync with what we use in Github actions

matyasf · 2025-08-18T14:45:14Z

packages/ui-codemods/lib/__node_tests__/runTest.ts

    if (entry.isFile() && entry.name.includes('input')) {
      const isWarningTest = entry.name.includes('.warning.input')
-      const inputPath = path.join(entry.path, entry.name)
+      const inputPath = path.join(entry.parentPath, entry.name)


entry.path is removed in Node 24, this is the recommended way since Node 20

matyasf · 2025-08-18T14:46:12Z

package.json

    "@types/react-dom": "^18.3.0",
    "@vitejs/plugin-react": "^4.5.1",
    "@vitest/eslint-plugin": "^1.2.1",
+    "@types/node": "^22",


This must be added otherwise it will just pick up a @types/node version from a dependency

Copilot

Pull Request Overview

This pull request adds extensive regression tests for UI components and includes bug fixes for SSR and accessibility issues. The purpose is to expand test coverage for visual regression testing in a Next.js application environment, focusing on documenting and testing happy path scenarios for UI components.

Key changes include:

Addition of 26+ new regression test pages covering major UI components
Fix for SSR hydration mismatch in Calendar component's weekday header IDs
Accessibility role corrections in Drilldown and Options components

Reviewed Changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
regression-test/src/app/*/page.tsx	New test pages for comprehensive component coverage including View, TreeBrowser, Tabs, Table, Select, and many others
packages/ui-calendar/src/Calendar/index.tsx	Fixed SSR issue by using deterministic IDs for weekday headers instead of random UIDs
packages/ui-drilldown/src/Drilldown/index.tsx	Fixed accessibility by changing default role from 'button' to 'menuitem' for subpage navigation
packages/ui-options/src/Options/Item/styles.ts	Improved accessibility contrast for disabled states
regression-test/cypress/e2e/spec.cy.ts	Updated test suite with new component test cases and improved assertion

Comments suppressed due to low confidence (1)

regression-test/src/app/drilldown/page.tsx:162

[nitpick] Using inline styles with opacity: 0 to hide icons is not the best practice. Consider using a proper placeholder component or conditional rendering instead of invisible elements.

            renderBeforeLabel={<IconCheckSolid style={{ opacity: 0 }} />}

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

regression-test/src/app/small-components/page.tsx

balzss

there are some strange things with text inputs:

they don't display focusrings in storybook mode (other components, e.g. buttonns or checkboxes do)
textarea has the right and left side cut off:

left padding seems to be missing?

balzss · 2025-08-21T06:15:06Z

regression-test/cypress/e2e/spec.cy.ts

    cy.checkA11y('.axe-test', axeOptions, terminalLog)
  })

+  it('check badge', () => {


please remove "check" from test names so it is cleaner (esp. in chromatic ui)

balzss · 2025-08-21T06:16:59Z

regression-test/src/app/button/page.tsx

+          </Flex.Item>
+        </Flex>
+      </View>
+      <div style={{ display: 'flex', gap: '0.5rem' }}>


these iconbuttons look "squashed" in chromatic:

I could fix this with a small delay

matyasf · 2025-08-22T14:58:34Z

there are some strange things with text inputs:

they don't display focusrings in storybook mode (other components, e.g. buttons or checkboxes do)

textArea has the right and left side cut off

+1: Text input fields dont apply inner padding.

I could not figure out why this is happening, they look good in Cypress. For some reason the CSS is not applied fully in Storybook, could be an SSR issue? I will make a ticket and fix it in the future

in Calendar the weekday header IDs were getting out of sync because they were not deterministically generated

balzss

everything looks good (except the input fields which you mentioned will be addressed separately, if you create a ticket, please include the fact that the left and right sides of the textarea on form errors are cut off)

the disabled button is correct again, maybe because of the delay?

the github action fails with an npm install error which I've not seen before, please take a look at that, otherwise everything is ok 👍

matyasf self-assigned this Aug 14, 2025

matyasf changed the base branch from master to more_regression_tests August 14, 2025 14:35

matyasf force-pushed the even_more_regression_tests branch 2 times, most recently from a30e7e4 to 3f688e1 Compare August 15, 2025 10:25

matyasf commented Aug 15, 2025

View reviewed changes

Base automatically changed from more_regression_tests to master August 18, 2025 10:26

matyasf force-pushed the even_more_regression_tests branch from 3f688e1 to 5011c0e Compare August 18, 2025 13:12

matyasf commented Aug 18, 2025

View reviewed changes

matyasf force-pushed the even_more_regression_tests branch from 5011c0e to 46be39d Compare August 18, 2025 13:18

matyasf commented Aug 18, 2025

View reviewed changes

matyasf force-pushed the even_more_regression_tests branch from 46be39d to 064a01c Compare August 18, 2025 14:43

matyasf commented Aug 18, 2025

View reviewed changes

matyasf force-pushed the even_more_regression_tests branch 6 times, most recently from 9570270 to 9df41e3 Compare August 19, 2025 08:35

matyasf requested review from HerrTopi, balzss and Copilot August 19, 2025 08:39

Copilot AI reviewed Aug 19, 2025

View reviewed changes

regression-test/src/app/small-components/page.tsx Show resolved Hide resolved

regression-test/src/app/small-components/page.tsx Show resolved Hide resolved

matyasf force-pushed the even_more_regression_tests branch from 9df41e3 to 217e194 Compare August 19, 2025 08:41

matyasf requested a review from joyenjoyer August 19, 2025 09:01

matyasf changed the title ~~[WIP] Even more regression tests~~ Add the (almost complete) regression test suite, fix small a11y issues, SSR errors Aug 19, 2025

matyasf changed the title ~~Add the (almost complete) regression test suite, fix small a11y issues, SSR errors~~ INSTUI-4672 Add the (almost complete) regression test suite, fix small a11y issues, SSR errors Aug 19, 2025

balzss requested changes Aug 21, 2025

View reviewed changes

matyasf force-pushed the even_more_regression_tests branch 3 times, most recently from 220c1c0 to cda82e4 Compare August 22, 2025 14:57

matyasf requested a review from balzss August 22, 2025 14:58

test: add more regression tests. Fix SSR error in Calendar

b3d442d

in Calendar the weekday header IDs were getting out of sync because they were not deterministically generated

matyasf force-pushed the even_more_regression_tests branch from cda82e4 to b3d442d Compare August 28, 2025 13:54

balzss approved these changes Aug 28, 2025

View reviewed changes

HerrTopi approved these changes Aug 28, 2025

View reviewed changes

matyasf merged commit b94c875 into master Aug 29, 2025
11 of 12 checks passed

matyasf deleted the even_more_regression_tests branch August 29, 2025 08:35

Conversation

matyasf commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matyasf Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matyasf Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matyasf Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

balzss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matyasf commented Aug 22, 2025

Uh oh!

balzss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matyasf commented Aug 14, 2025 •

edited

Loading

github-actions bot commented Aug 14, 2025 •

edited

Loading

matyasf Aug 15, 2025 •

edited

Loading

matyasf Aug 18, 2025 •

edited

Loading

matyasf Aug 18, 2025 •

edited

Loading