Skip to content

Conversation

@gitdallas
Copy link
Contributor

@gitdallas gitdallas commented May 11, 2023

will close #8993

about modal (larger X is back, backgrounds are gone) https://patternfly-react-pr-9108.surge.sh/components/about-modal

@@ -1,5 +1,5 @@
import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/ChipGroup/chip-group';
import styles from '@patternfly/react-styles/css/components/Chip/chip-group';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to update this because of this change: patternfly/patternfly#5559

@gitdallas gitdallas requested a review from nicolethoen May 11, 2023 17:55
@patternfly-build
Copy link
Contributor

patternfly-build commented May 11, 2023

@gitdallas gitdallas requested a review from mcoker May 11, 2023 18:21
@gitdallas
Copy link
Contributor Author

gitdallas commented May 11, 2023

@mcoker - do the snapshot updates make sense with recent core changes? inset classes being removed?

@mcoker
Copy link
Contributor

mcoker commented May 11, 2023

@gitdallas nope, glad you caught that. It's a bug in core, @mattnolting and I are looking at it now - patternfly/patternfly#5560

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

It seems like this does resolve the original issue.

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

Wait it looks like an integration test is failing, is that just related to the other core bug mentioned above?

@mcoker
Copy link
Contributor

mcoker commented May 11, 2023

@gitdallas @wise-king-sullyman this should be fixed now in 5.0.0-alpha.53

import { useState } from 'react';
import styles from '@patternfly/react-styles/css/components/Label/label';
import labelGrpStyles from '@patternfly/react-styles/css/components/LabelGroup/label-group';
import labelGrpStyles from '@patternfly/react-styles/css/components/Label/label-group';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

update based on pf core change

@@ -1,5 +1,5 @@
import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/LabelGroup/label-group';
import styles from '@patternfly/react-styles/css/components/Label/label-group';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

update based on pf core change

@@ -1,4 +1,4 @@
import { pfIcons } from '@patternfly/patternfly/icons/pf-icons.mjs';
import { pfIcons } from '@patternfly/patternfly/icons/pficons.mjs';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

update based on pf core change

@gitdallas
Copy link
Contributor Author

gitdallas commented May 12, 2023

@wise-king-sullyman - right. an issue was discovered with pf core while updating

@mcoker - I've had to make a few other changes (pf-icons => pficons, LabelGroup => Label), i'm not sure what to do with the background image changes. what do you think?

https://github.com/patternfly/patternfly-react/pull/9108/files#diff-8c26c106b002502c6b636b8af398bb5ac97fbfb9679673f9208dde8dfcfa7750R95

@gitdallas
Copy link
Contributor Author

Commented out background image related stuff, will be handled with this issue #9051

@wise-king-sullyman
Copy link
Contributor

@gitdallas looks like there's just a few unit test that probably just need to be .skiped for now, then I think this should be good.

@tlabaj tlabaj merged commit 06599d5 into patternfly:v5 May 15, 2023
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.

Bug - [About Modal] - [[x] button to close modal is very small and not matching the html demos]

6 participants