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 invalid property types #19505

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Conversation

martinpitt
Copy link
Member

No description provided.

@martinpitt martinpitt added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 19, 2023
@martinpitt martinpitt removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 19, 2023
In `Dialog`, the `error` property is always a string, as
`show_modal_dialog()`'s `set_error` call already stringifies objects. Hence
they can never actually have a `.details` field, and our code also does not set
this anywhere.

This fixes a lot of property type warnings.
@martinpitt martinpitt marked this pull request as ready for review October 19, 2023 10:03
@martinpitt martinpitt requested review from mvollmer and jelly October 19, 2023 10:03
pkg/lib/cockpit-components-dialog.jsx Outdated Show resolved Hide resolved
pkg/lib/cockpit-components-dialog.jsx Show resolved Hide resolved
@martinpitt martinpitt temporarily deployed to gh-cockpituous October 19, 2023 12:43 — with GitHub Actions Inactive
Dialog.title must be a `string`. PatternFly's modal has a
`titleIconVariant` property for this use case, which is exposed by our
Dialog component. Use that instead.

Adjust the pixel references for the slightly changed title layout.
The previous commits fix all of them.
@martinpitt
Copy link
Member Author

This slightly changes the dialog, pushed updated pixels.

@martinpitt martinpitt temporarily deployed to gh-cockpituous October 19, 2023 13:21 — with GitHub Actions Inactive
@martinpitt martinpitt requested a review from jelly October 19, 2023 13:21
Copy link
Member

@mvollmer mvollmer left a comment

Choose a reason for hiding this comment

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

Oh, our dialog even already exposed titleIconVariant! Thanks!

@martinpitt martinpitt merged commit 6a5b725 into cockpit-project:main Oct 19, 2023
36 checks passed
@martinpitt martinpitt deleted the prop-types branch October 19, 2023 14: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.

3 participants