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

lib: Drop unknown noVerticalAlign property from HelpIcon #19537

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

martinpitt
Copy link
Member

This isn't documented anywhere on https://www.patternfly.org/components/icon/, no other instance uses it, and React complains about it:

React does not recognize the noVerticalAlig prop on a DOM element.
We don't cover that bit in Cockpit's tests, but cockpit-machines does.

This isn't documented anywhere on https://www.patternfly.org/components/icon/,
no other instance uses it, and React complains about it:

> React does not recognize the `noVerticalAlig` prop on a DOM element.
We don't cover that bit in Cockpit's tests, but cockpit-machines does.
@martinpitt martinpitt added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 27, 2023
@martinpitt
Copy link
Member Author

@KKoukiou You introduced that property, maybe you can still remember why?

@@ -99,7 +99,7 @@ export const PasswordFormFields = ({
<Popover bodyContent={password_label_info}>
<button onClick={e => e.preventDefault()}
className="pf-v5-c-form__group-label-help">
<HelpIcon noVerticalAlign />
<HelpIcon />
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@jelly
Copy link
Member

jelly commented Oct 27, 2023

In Machines we dropped this before cockpit-project/cockpit-machines@4cad663 and no pixel tests changed so let's merge it.

@martinpitt martinpitt merged commit 0f31099 into cockpit-project:main Oct 27, 2023
41 of 42 checks passed
@martinpitt martinpitt deleted the helpicon branch October 27, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-test For doc/workflow changes, or experiments which don't need a full CI run,
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants