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

Refactor pattern component wrappers #421

Merged
merged 22 commits into from
Jan 13, 2025
Merged

Conversation

danielnaab
Copy link
Contributor

@danielnaab danielnaab commented Dec 23, 2024

For #359, handle wrapping PromptComponent objects with React components in the component itself, rather than in the form mechanics.

A number of issues were encountered while updating this, which led to upgrading several dependencies, including Vite, Vitest, and Storybook. This enabled removing JSDOM usage from the Storybook tests and the describeStories usage in favor of Storybook new "experimental" usage of Vitest browser mode. Additionally, node.js was updated to the current-latest LTS, v22.12.0.

…comonent itself, rather than in the form mechanics.
Copy link

github-actions bot commented Dec 23, 2024

Terraform plan for tts-10x-atj-dev

Plan: 0 to add, 2 to change, 0 to destroy.
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~  update in-place

Terraform will perform the following actions:

  # cloudfoundry_app.tts-10x-atj-dev-server-doj_tts-10x-atj-dev-server-doj-app_380DB029 will be updated in-place
!~  resource "cloudfoundry_app" "tts-10x-atj-dev-server-doj_tts-10x-atj-dev-server-doj-app_380DB029" {
!~      docker_image                    = "ghcr.io/gsa-tts/forms/server-doj:b355f84dd5bd7cf8779360db7b238b6b5185856c" -> "ghcr.io/gsa-tts/forms/server-doj:7cbfe5b4707fef66acd948a41d4e8e5a38ea6a00"
        id                              = "8a9fc8b6-af5e-45a2-abb6-2c24ecbcdfaa"
        name                            = "tts-10x-atj-dev-server-doj-app"
#        (17 unchanged attributes hidden)

#        (3 unchanged blocks hidden)
    }

  # cloudfoundry_app.tts-10x-atj-dev-server-kansas_tts-10x-atj-dev-server-kansas-app_337A9CF1 will be updated in-place
!~  resource "cloudfoundry_app" "tts-10x-atj-dev-server-kansas_tts-10x-atj-dev-server-kansas-app_337A9CF1" {
!~      docker_image                    = "ghcr.io/gsa-tts/forms/server-kansas:b355f84dd5bd7cf8779360db7b238b6b5185856c" -> "ghcr.io/gsa-tts/forms/server-kansas:7cbfe5b4707fef66acd948a41d4e8e5a38ea6a00"
        id                              = "e885e531-11b7-4906-9cc3-0ddf483868f5"
        name                            = "tts-10x-atj-dev-server-kansas-app"
#        (17 unchanged attributes hidden)

#        (3 unchanged blocks hidden)
    }

Plan: 0 to add, 2 to change, 0 to destroy.

📝 Plan generated in Post Terraform plan to PR comment #596

@danielnaab danielnaab force-pushed the child-rendering-refactor branch from 7de568d to bfb5e40 Compare January 8, 2025 17:01
@danielnaab danielnaab force-pushed the child-rendering-refactor branch from b7f28ac to b644328 Compare January 10, 2025 17:11

// Press the spacebar to exit reordering mode
await userEvent.type(grabber, ' ');
await new Promise(r => setTimeout(r, 100));
await new Promise(r => setTimeout(r, 1000));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really didn't want to update these, but couldn't find a better way to get the tests to pass.

@@ -15,7 +15,7 @@
"format": "prettier --write \"packages/*/src/**/*.{js,jsx,ts,tsx,scss,css}\"",
"lint": "turbo run lint",
"pages": "rm -rf node_modules && npm i -g pnpm turbo && pnpm i && pnpm build && ln -sf ./apps/spotlight/dist _site",
"test": "vitest run && pnpm --filter @atj/design test:ci",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vitest.workspace.ts now includes these tests.

"import": "./dist/esm/index.js",
"require": "./dist/cjs/index.js",
"types": "./dist/types/index.d.ts"
"require": "./dist/cjs/index.js"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of these keys became relevant with node 22.

@@ -5,8 +5,8 @@ import commonjs from '@rollup/plugin-commonjs';
import json from '@rollup/plugin-json';
import typescript from 'rollup-plugin-typescript2';

import packageJson from './package.json' assert { type: 'json' };
import workspacePackageJson from '../../package.json' assert { type: 'json' };
import packageJson from './package.json' with { type: 'json' };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For node.js 22 compatibility

@@ -18,6 +18,7 @@ const config: StorybookConfig = {
getAbsolutePath('@storybook/addon-interactions'),
getAbsolutePath('@storybook/addon-a11y'),
getAbsolutePath('@storybook/addon-coverage'),
getAbsolutePath('@storybook/experimental-addon-test'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will hopefully come out of "experimental" status soon, but seems to be running pretty smoothly now.

@@ -12,8 +13,7 @@ const Fieldset: PatternComponent<FieldsetProps> = props => {
{props.legend}
</legend>
)}

{props.children}
{renderPromptComponents(props.context, props.childComponents)}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the components that have children have been updated to call a function to render the child prompt component parts. This will enable rendering components with alternate preview props in the edit UI, which will be necessary for the logic UI.

@danielnaab danielnaab marked this pull request as ready for review January 10, 2025 17:30
Copy link
Contributor

@kalasgarov kalasgarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a small comment. LGTM! Great work! 🚀

@danielnaab danielnaab merged commit bc1a1b6 into main Jan 13, 2025
4 checks passed
@danielnaab danielnaab deleted the child-rendering-refactor branch January 13, 2025 22:44
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