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.

Sign up for GitHub

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

Jump to bottom

Add Storybook to simplify future UI development #27

Merged
merged 16 commits into from
Dec 9, 2023
Merged

Add Storybook to simplify future UI development #27

merged 16 commits into from
Dec 9, 2023

Conversation

markafitzgerald1
Copy link
Owner

Only dependency updates in here so far.

@markafitzgerald1
build(deps): move to latest + automate LTS usage
5b345b5 
Includes update of README.md instructions and GitHub Actions to always
use latest LTS version of Node.js.
@markafitzgerald1
🔨 build(deploy): only deploy from main branch
418d944
@markafitzgerald1 markafitzgerald1 self-assigned this Oct 28, 2023
@markafitzgerald1 markafitzgerald1 marked this pull request as ready for review October 28, 2023 17:55
@markafitzgerald1 markafitzgerald1 marked this pull request as draft October 28, 2023 18:00
@markafitzgerald1 markafitzgerald1 mentioned this pull request Oct 28, 2023
Closed
@markafitzgerald1
💾 chore(server+bundler): Replace Parcel with Vite
633c7d6 
Done to facilitate planned use of Storybook. Side benefit is to use a
more performant and actively maintained dev server and bundler.
@markafitzgerald1
🤖 test (Add Storybook and Card stories)
4281a13 
Using Storybook to simplify addition of analytics user consent user
interface to the application.
@markafitzgerald1
🤖 test (Update Linux Playwright test screenshots)
fd26287
@markafitzgerald1
🔨 build: Remove unused dependency
8aaa89a
@markafitzgerald1
🤖 test(Storybook): Use test runner
81f6df8
@markafitzgerald1
🔨 build: Run Storybook before its tests
3ad9eaf
@markafitzgerald1
🔨 build(Storybook): Install browsers before tests
cf48dfc
@markafitzgerald1
🔨 build(local): Ensure Storybook up for e2e tests
cdc6d63 
Local development was counting on the developer separately running `npm
run storybook`.  Also removed Docker port mapping that was not working
- local serve of Playwright report via `npx --no-install playwright
show-report` delivers what was intended by that port mapping.
@markafitzgerald1
🚀 ci(e2e tests): Restore tests on GitHub
be1224d 
Previous change caused e2e tests to not run in GitHub Actions.
@markafitzgerald1
🔨 build: Restore Storybook serve before e2e tests
8d00ddc
@markafitzgerald1
🐞 fix(Card): Minimize per components props
822cfae 
Each Card React component used to know about and modify the entire
hand. Now each Card only knows about itself and fires events for the
owner of hand state to modify state instead.
@markafitzgerald1
🤖 test(CardLabel): Add stories
127963b
@markafitzgerald1
Copy link
Owner Author

At 25,000 lines, this PR has gotten to big to meaningfully review. Given that the branch of this PR is effectively the only tip of mainline development and it is deployable, it should probably be merged into main while work on issue #21 continues.

@markafitzgerald1
Copy link
Owner Author

Based on initial review, it might be better to instead reimplement this PR piecemeal in main to reduce risk as the intents of some of the changes in this PR are no longer immediately clear to me. It is possible that some commits on the branch were made with the intent that they eventually be PR reviewed, but as previously noted, this PR is now too large to review.

@markafitzgerald1 markafitzgerald1 marked this pull request as ready for review December 9, 2023 15:46
@markafitzgerald1
Copy link
Owner Author

This PR and branch is now equal to main, thus closing the PR. Will switch to trunk-based development for now, then perhaps go back to PR required to get into main (see #33), but with much smaller PRs!

@markafitzgerald1
Copy link
Owner Author

Close attempt complained about unmerged commits, so will merge to silence that warning (and keep the original history of this PR).

@markafitzgerald1 markafitzgerald1 merged commit 0ce094d into main Dec 9, 2023
2 checks passed
@markafitzgerald1 markafitzgerald1 deleted the 21-measure-cribbage-trainer-github-pages-usage branch December 9, 2023 15:56
@markafitzgerald1 markafitzgerald1 changed the title Measure Cribbage Trainer GitHub pages usage Add Storybook to simplify future UI development Dec 9, 2023
@markafitzgerald1
Copy link
Owner Author

Title changed from "Measure Cribbage Trainer GitHub pages usage" to "Add Storybook to simplify future UI development" as the PR didn't end up doing all of the work to resolve #21.

@markafitzgerald1 markafitzgerald1 added enhancement New feature or request dependencies Pull requests that update a dependency file labels Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@markafitzgerald1 markafitzgerald1

Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@markafitzgerald1