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

Theme-defined backgrounds #6310

Merged

Conversation

halocline
Copy link
Collaborator

@halocline halocline commented Aug 30, 2022

What does this PR do?

Closes #6291

  • Allows a caller to specify a theme-defined background such as <Box background="gradient-1" ... /> or <Box background="image-3" ... />.
  • Extends the background prop object to support background-clip.
  • Extends the background prop object to support rotate, specified in degrees. Note: Implementation only supports rotation of linear-gradients at this time. Proper support for rotating images requires image resizing and repositioning and would need to be thought through from an API perspective.

Backgrounds may be a string, {dark, light} object, or BackgroundType object.

const myTheme = {
  global: {
    backgrounds: {
      'image-1': `url(https://images.unsplash.com/photo-1620497772559-b65fc1d1ccf7?ixlib=rb-1.2.1&ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&auto=format&fit=crop&w=687&q=80)`,
      'image-2': {
        dark: `url(https://images.unsplash.com/photo-1614292253389-bd2c1f89cd0e?ixlib=rb-1.2.1&ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&auto=format&fit=crop&w=687&q=80)`,
        light: `url(https://images.unsplash.com/photo-1603484477859-abe6a73f9366?ixlib=rb-1.2.1&ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&auto=format&fit=crop&w=687&q=80)`,
      },
      'gradient-1': `linear-gradient(
        hsl(240deg 90% 55%) 0%,
        hsl(341deg 90% 55%) 50%,
        hsl(60deg 90% 55%) 100%)`,
    },
  },
};

Where should the reviewer start?

background.js

What testing has been done on this PR?

  • Storybook
  • Jest test suite
  • New Jest tests --> in progress, getting an odd, difficult to trace error coming from Jest.

How should this be manually tested?

Storybook --> Use the Box/stories/TEMP.js story

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6291

Screenshots (if appropriate)

Screen Shot 2022-09-09 at 4 07 38 PM

Do the grommet docs need to be updated?

Yes.

Should this PR be mentioned in the release notes?

Yes.

Is this change backwards compatible or is it a breaking change?

Backwards compatible.

@halocline
Copy link
Collaborator Author

Fix coming for the Jest tests.

Copy link
Collaborator

@MikeKingdom MikeKingdom left a comment

Choose a reason for hiding this comment

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

Cool stuff

src/js/utils/background.js Outdated Show resolved Hide resolved
src/js/utils/background.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@MikeKingdom MikeKingdom left a comment

Choose a reason for hiding this comment

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

Looks great. It's a pretty cool feature.

Copy link
Collaborator

@britt6612 britt6612 left a comment

Choose a reason for hiding this comment

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

LGTM! not sure if you wanted to make a separate issue for the opacity?

Copy link
Collaborator

@taysea taysea left a comment

Choose a reason for hiding this comment

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

Nice work on this, good additions.

src/js/utils/background.js Outdated Show resolved Hide resolved
src/js/utils/background.js Show resolved Hide resolved
src/js/themes/base.d.ts Show resolved Hide resolved
@halocline halocline marked this pull request as draft August 31, 2022 18:25
@halocline halocline marked this pull request as ready for review September 9, 2022 22:12
@ericsoderberghp
Copy link
Contributor

@halocline
Copy link
Collaborator Author

@ericsoderberghp ericsoderberghp merged commit 6a9b3c9 into grommet:master Sep 13, 2022
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.

Enhance theme to support theme-defined backgrounds
5 participants