-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update Dialog to use children #907
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No hard blockers but I think at least some of the comments, like cleaning up the dead CSS, should be done within this PR.
apps/cyberstorm-storybook/stories/components/Dialog.stories.tsx
Outdated
Show resolved
Hide resolved
...rm/src/components/Layout/PackageDetailLayout/PackageManagementForm/PackageManagementForm.tsx
Outdated
Show resolved
Hide resolved
packages/cyberstorm/src/components/Layout/Teams/TeamSettings/TeamMembers/TeamMemberList.tsx
Outdated
Show resolved
Hide resolved
packages/cyberstorm/src/components/Layout/Teams/TeamSettings/TeamMembers/TeamMembers.module.css
Outdated
Show resolved
Hide resolved
packages/cyberstorm/src/components/Layout/Teams/TeamSettings/TeamMembers/TeamMembers.tsx
Outdated
Show resolved
Hide resolved
...ents/Layout/Teams/TeamSettings/TeamServiceAccounts/ServiceAccountList/ServiceAccountList.tsx
Show resolved
Hide resolved
590086d
to
f193c39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the any
type and CSS nth-child
bug (unless it's just misunderstanding on my part) should be fixed. Also check the older comments for replies.
import { PackageDependencyDialog } from "./PackageDependencyDialog/PackageDependencyDialog"; | ||
import * as Button from "../../../Button/"; | ||
import { useDapper } from "@thunderstore/dapper"; | ||
import { usePromise } from "@thunderstore/use-promise"; | ||
import { Dialog } from "../../../.."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest changes brought multiple of these import back the PR. IDK if the tooling for autoimports could be improved to resolve this. Until then I'd prefer we'd fix these manually. Not a blocker though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems they are not all fixed yet.
3c1095e
to
5ae34bd
Compare
refs TS-1955
refs TS-1955
5ae34bd
to
d5b9ea5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of including fixes I requested in another PR in this PR.
FWIW there's still one open comment from earlier review.
refs TS-1955