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

chore: make the development process easier with Storybook #541

Merged

Conversation

adguernier
Copy link
Contributor

@adguernier adguernier commented Mar 27, 2024

Q A
Branch? main for features
Tickets Following #535
License MIT
Doc PR WIP

This PR attempts to validate that passing a React-admin authProvider to the <HydraAdmin> component as a prop works well.

It adds some Stories and interactions tests running thanks to Storybook and Playwright.

You might read the CONTRIBUTING.md first to know how to test this PR.

Todo:

  • add stories to demonstrate the authProvider prop
  • add E2E tests
  • update CONTRIBUTING.md file

@adguernier adguernier added the RFR label Apr 8, 2024
@slax57 slax57 self-requested a review April 9, 2024 14:52
.github/workflows/storybook.yml Outdated Show resolved Hide resolved
.github/workflows/storybook.yml Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
src/stories/auth/Admin.tsx Outdated Show resolved Hide resolved
src/stories/auth/basicAuth.ts Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Huge work, thank you!

I made some minor comments, otherwise, LGTM.

.gitignore Outdated Show resolved Hide resolved
.yarnrc.yml Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
api/bin/phpunit Outdated Show resolved Hide resolved
api/composer.json Show resolved Hide resolved
compose.ci.yaml Outdated Show resolved Hide resolved

CMD ["yarn", "serve"]
RUN set -eux; \
yarn playwright install --with-deps && yarn cache clean
Copy link
Member

Choose a reason for hiding this comment

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

should we install this on the dev container as well? then add instructions on how to run that inside docker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


# Namespaced pools use the above "app" backend by default
#pools:
#my.dedicated.cache: null
Copy link
Member

@soyuka soyuka May 2, 2024

Choose a reason for hiding this comment

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

can you remove the comments in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,62 @@
monolog:
Copy link
Member

Choose a reason for hiding this comment

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

I wish we could remove this, I'll see that in another patch

Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

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

nice!

@adguernier adguernier changed the title Chore: add stories and test to validate that authProvider works chore: make the development easier with Storybook May 6, 2024
@adguernier adguernier changed the title chore: make the development easier with Storybook chore: make the development process easier with Storybook May 6, 2024
@adguernier adguernier merged commit 551ba4c into api-platform:main May 6, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants