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: Web Forms Preview demo form links #265

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Conversation

eyelidlessness
Copy link
Member

I have verified this PR works in these browsers (latest versions):

  • Chrome
  • Firefox
  • Safari (macOS)
  • Safari (iOS)
  • Not applicable

What else has been done to verify that this works as intended?

Added an e2e test which loads all of the Web Forms Preview form links, from the build product. That should be much more reliable than any manual testing I might do.

Why is this the best possible solution? Were any other approaches considered?

It would be preferable if specific fixtures could be referenced by something stable, and not rely on anything dynamic at build time.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

It's possible some other nuance of glob loading is affected by this. But I would expect to see some sign of that either in scenario or dev mode, both of which still seem to be working as expected.

Do we need any specific form for testing your changes? If so, please attach one.

The links on the preview page should suffice.

What's changed

  • Restored use of build-time asset URLs in links to XForm fixtures
  • Added an e2e test to ensure that doesn't break again, along with a couple of basic Vue template supports for (hopefully) clearer intent in e2e tests going forward

The nature of this fix is that Vite’s `import.meta.glob` produces a build-time asset URL, which we used previously, but missed in the generalization of glob import loading. This was missed because the dev/test asset URL is identical to the relative path key which we did use in that generalization.

Since this implicates an important difference betwen dev/test and build, an e2e test will follow to exercise loading demo forms directly from the Web Forms Preview build product.
Includes a couple of small template changes:

- Explicit selector for identifying the links themselves
- Selector for identifying when a form has completed loading, and what its loading status is

The test itself checks for either an error or ready status, and then explicitly checks that no error message is displayed. I think this makes the test’s _intent_ clear enough that it’ll hopefully be maintainable longer term (whereas I struggle to understand some of the existing e2e tests when I look back at them after any length of time).
Copy link

changeset-bot bot commented Dec 16, 2024

⚠️ No Changeset found

Latest commit: b0f19c3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@eyelidlessness
Copy link
Member Author

I don't think this needs a changeset? It should hopefully only affect the Web Forms Preview secondary build product.

@eyelidlessness eyelidlessness changed the title Fix/post 060 wfp Fix: Web Forms Preview demo form links Dec 16, 2024
@eyelidlessness eyelidlessness merged commit dfc6c70 into main Dec 16, 2024
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants