Skip to content

Conversation

@jpuzz0
Copy link
Contributor

@jpuzz0 jpuzz0 commented May 26, 2023

What: Closes #9180

@jpuzz0 jpuzz0 requested review from gitdallas, mcarrano and tlabaj May 26, 2023 14:22
@patternfly-build
Copy link
Contributor

patternfly-build commented May 26, 2023

@tlabaj tlabaj requested a review from mcoker May 26, 2023 14:46
@jpuzz0 jpuzz0 force-pushed the fix/background-image branch from 5f734ca to 2bcc8ed Compare May 26, 2023 15:34
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

lgtm

/* eslint-enable camelcase */
style={
{
'--pf-v5-c-background-image--BackgroundImage': `url(${src})`
Copy link
Contributor

@gitdallas gitdallas May 30, 2023

Choose a reason for hiding this comment

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

it would be nice if we had a token/css prop for this - preventing future "replace hard coded values" work

@tlabaj - would it be worth combining with base css prop like --${styles.backgroundImage}--BackgroundImage?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @mcoker

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree - this PR should add a token you can set patternfly/patternfly#5633

Copy link
Contributor

Choose a reason for hiding this comment

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

@gitdallas @mcoker I think we can take this PR in as is and ope na follow up issue to the core PR.

@jpuzz0 jpuzz0 force-pushed the fix/background-image branch from 2bcc8ed to 2b23e5a Compare May 31, 2023 15:42
Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @jpuzz0 !

@tlabaj tlabaj merged commit 1f8a35b into patternfly:v5 Jun 1, 2023
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@5.0.0-alpha.129
  • @patternfly/react-core@5.0.0-alpha.128
  • @patternfly/react-docs@6.0.0-alpha.138
  • demo-app-ts@5.0.0-alpha.112
  • @patternfly/react-table@5.0.0-alpha.132

Thanks for your contribution! 🎉

nicolethoen pushed a commit to Kells512/patternfly-react that referenced this pull request Sep 1, 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 - Background image - still shows old background image

6 participants