feat(modal): add tokens to modal and about modal #6042

srambach · 2023-11-08T14:53:38Z

NOTE: The about modal image currently does not switch over in dark mode. We'll need a solution for this. Right now I've commented out a couple of lines that were being used for sizing the background image in favor of using "cover" but if we use a different background image we may want these back again. @lboehling what ideas do you have for the image, and what is the expectation as the modal is resized?

ALSO, this corrects a previous mistake in the line-height tokens, where the semantic tokens for regular text and heading line-height were switched.

patternfly-build · 2023-11-08T14:59:11Z

Preview: https://patternfly-pr-6042.surge.sh

A11y report: https://patternfly-pr-6042-a11y.surge.sh

lboehling · 2023-11-08T17:19:06Z

this is looking awesome 🎉

here's an svg for the bkg image

If we use the gray-40 fill color for both shapes and put them both at 15% opacity, it looks good on both our primary and secondary background colors in light and dark theme i think?

The image is resizing as i would expect in the PR!

mcoker

LGTM, left some comments. The only thing I might go ahead and update is setting the modal to overflow: auto, but that can be a follow up. Lemme know if you'd rather merge and follow up or update this PR.

mcoker · 2023-11-09T01:27:03Z

src/patternfly/components/AboutModalBox/about-modal-box.scss

  // Component variables
  --#{$about-modal-box}--BackgroundImage: none;
-  --#{$about-modal-box}--BackgroundColor: var(--#{$pf-global}--palette--black-1000); // Modal uses a non-standard background color
+  --#{$about-modal-box}--BackgroundColor: var(--pf-t--global--background--color--floating--default);


We could also remove this if the intent from design is that this comes from the modal. Similar to the rounded corners and box-shadow. The design would even be a lot more clear if the about modal were created inside of the modal component, so it's clear what parts of the design are from what components.

It wasn't clear to me whether this background color would simply be the modal background color or if them being the same was a coicidence. @lboehling ?

mcoker · 2023-11-09T01:42:18Z

src/patternfly/components/AboutModalBox/about-modal-box.scss

+  --#{$about-modal-box}--BackgroundSize--width: 100%;
+  --#{$about-modal-box}--BackgroundSize: contain;


We should nail down what the intent is with the background image - @lboehling @andrew-ronaldson do we have a design guideline for what kind of image to use here? Just our ideal intent - what size, where would it go in relation to whatever else is on the page, how much of the page should it span, etc. FYI if you put a wide image in now, with contain, it will fill the whole box - https://codepen.io/mcoker/pen/PoVpVqQ

Doesn't need to hold up this PR, just something to consider. Also the about modal and background image component CSS is basically the same. The login page uses the background image component - seems like the about modal could, too, and we could just remove the background image specific styles from the about modal?

I took out the sizing because it's fairly specific to the image anyway and this one needs to span the full height or it will look odd. And there will need to be guidelines on making the image since it needs to handle the switch to dark theme.
I do think we should probably just use the background component here?

mcoker · 2023-11-09T01:43:30Z

src/patternfly/components/AboutModalBox/about-modal-box.scss

-  --#{$about-modal-box}__close--c-button--Height: calc(var(--#{$about-modal-box}__close--c-button--FontSize) * 2);
-  --#{$about-modal-box}__close--c-button--BackgroundColor: var(--#{$pf-global}--palette--black-1000); // Close button uses a non-standard background color
-  --#{$about-modal-box}__close--c-button--hover--BackgroundColor: #{rgba($pf-v5-color-black-1000, .4)};
+  --#{$about-modal-box}__close--c-button--Color: var(--pf-t--global--icon--color--regular);


You shouldn't need this - it will come from the button.

mcoker · 2023-11-09T01:48:01Z

src/patternfly/components/AboutModalBox/about-modal-box.scss

    display: flex;
    align-items: center;
    justify-content: center;
-    width: var(--#{$about-modal-box}__close--c-button--Width);
-    height: var(--#{$about-modal-box}__close--c-button--Height);
    padding: 0;
    font-size: var(--#{$about-modal-box}__close--c-button--FontSize);
    color: var(--#{$about-modal-box}__close--c-button--Color);
-    background-color: var(--#{$about-modal-box}__close--c-button--BackgroundColor);
-    border-radius: var(--#{$about-modal-box}__close--c-button--BorderRadius);
-
-    &:hover {
-      --#{$about-modal-box}__close--c-button--BackgroundColor: var(--#{$about-modal-box}__close--c-button--hover--BackgroundColor);
-    }


I think you can get rid of all of this custom CSS aside from the font-size. Everything else should just come from the plain button. All of this is because it is a circle shape in PF4/5.

mcoker · 2023-11-09T01:58:19Z

src/patternfly/components/ModalBox/modal-box.scss

  width: var(--#{$modal-box}--Width);
  max-width: var(--#{$modal-box}--MaxWidth);
  max-height: var(--#{$modal-box}--MaxHeight);
+  overflow: hidden;


If you have a tall thing in the modal and no modal body (the only element that allows you to scroll overflow currently), it will just run off the screen. I would probably set this to auto instead.

This may cause a double scrollbar as a tall modal with a tall body element is in a super short viewport, but we can revisit that. I don't know that it would be any worse than it is now. We could also remove the overflow scroll from the modal body and make the footer sticky to the bottom.

srambach · 2023-11-21T20:56:20Z

@lboehling says the description is not actually part of an about modal, so @mcoker please let me know if there are any additional things - I fixed the close button and spacing.

mcoker

🤖👍

patternfly-build · 2023-11-21T21:49:49Z

🎉 This PR is included in version 6.0.0-alpha.20 🎉

The release is available on:

Your semantic-release bot 📦🚀

* feat(breaker): bump this version to 6.0 (#5800) * fix: use prerelease tag format with dry run * fix: turn off dryrun * feat(tokens): add initial global tokens (#5843) * feat(Button): adds tokens and applies to buttons (#5908) * feat(table): apply tokens from hackathon (#5965) * feat(menu-toggle): add in work from hackathon (#5931) * feat(nav): adds tokens to page and nav (#5942) * chore(tokens): update tokens from figma (#5970) * feat(page): page and masthead updates from hackathon (#5968) * fix(tokens): add dark theme blend mode token (#5992) * feat(notificationBadge): refactor with tokens (#5991) * chore(masthead ): applied tokens (#6004) * feat(alert): apply tokens (#6002) * feat(alert): apply tokens * feat(alert): remove unused background color * feat(page): apply tokens (#6021) * feat(page): apply tokens * feat(page): undo menu__list change * feat(page): tweak spacing * fix(table): fix clickable overlay (#6025) * fix(box-shadow): typo in box shadow token names (#6039) * fix(badge): adds tokens, dropdown toggle icon (#6037) * feat(hint): apply tokens (#6030) * feat(hint): apply tokens * feat(hint): fix typo * feat(hint): update spacing * feat(hint): update border radius * feat(hint): fix heading font * feat(text, title, list): apply tokens (#6031) * feat(page): remove padding on nav sidebar (#6053) * fix(tokens): adjust font-weight base for headings (#6070) * feat(login): apply tokens (#6028) * feat(modal): add tokens to modal and about modal (#6042) * fix(font): use variable font, add opt-in for legacy font (#6073) * feat(ExpandableSection): added tokens (#6063) * feat(Tooltip): applied tokens (#6064) * feat(Popover): applied tokens (#6065) * feat(BackToTop): refactor with tokens (#6085) * feat(SkipToContent): refactor with tokens (#6090) * feat(Accordion): applied tokens (#6058) * feat(banner): refactor with tokens (#6081) * feat(InlineEdit): applied tokens Took a stab at applying the tokens here. There are some items that will be fixed with icon buttons. * chore(tokens): updates tokens from figma (#6103) * chore(tokens): updating usage of new font tokens (#6111) * feat(breadcrumb): apply tokens (#6060) * feat(HelperText): apply tokens (#6118) * feat(HelperText): first tokens * feat(HelperText): tokens changes * feat(HelperText): fixes * feat(Pagination): applied tokens (#6089) * feat(Pagination): applied tokens * Removed themes directory * Removed button and menutoggle styles, added wrapper * Removed padding vars for pagination container * Updated tokens and vars for padding * fix(Pagination): updated property order * Updated var names to remove md text * feat(avatar): apply tokens (#6122) * chore(ExpandableSection): updated Penta docs and focus/hover styling (#6124) * feat(EmptyState): apply tokens (#6056) * fix(form): keep form groups/controls from growing too wide (#6109) * chore(deps): update dependency @patternfly/react-core to v5.1.2 (#6113) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency @patternfly/react-code-editor to v5.1.2 (#6112) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency @patternfly/react-table to v5.1.2 (#6114) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * fix(card/menu-toggle/check): alignment bugs (#6110) * feat(SimpleList): pull in new v6 branch (#6128) * chore: turn on dryrun until release fixed [ci skip] --------- Co-authored-by: Sarah Rambacher <srambach@redhat.com> Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com> Co-authored-by: Matt Nolting <matthew.nolting@gmail.com> Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com> Co-authored-by: Austin Sullivan <ausulliv@redhat.com> Co-authored-by: Andrew Ronaldson <aronalds@redhat.com> Co-authored-by: kmcfaul <45077788+kmcfaul@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* feat(breaker): bump this version to 6.0 (patternfly#5800) * fix: use prerelease tag format with dry run * fix: turn off dryrun * feat(tokens): add initial global tokens (patternfly#5843) * feat(Button): adds tokens and applies to buttons (patternfly#5908) * feat(table): apply tokens from hackathon (patternfly#5965) * feat(menu-toggle): add in work from hackathon (patternfly#5931) * feat(nav): adds tokens to page and nav (patternfly#5942) * chore(tokens): update tokens from figma (patternfly#5970) * feat(page): page and masthead updates from hackathon (patternfly#5968) * fix(tokens): add dark theme blend mode token (patternfly#5992) * feat(notificationBadge): refactor with tokens (patternfly#5991) * chore(masthead ): applied tokens (patternfly#6004) * feat(alert): apply tokens (patternfly#6002) * feat(alert): apply tokens * feat(alert): remove unused background color * feat(page): apply tokens (patternfly#6021) * feat(page): apply tokens * feat(page): undo menu__list change * feat(page): tweak spacing * fix(table): fix clickable overlay (patternfly#6025) * fix(box-shadow): typo in box shadow token names (patternfly#6039) * fix(badge): adds tokens, dropdown toggle icon (patternfly#6037) * feat(hint): apply tokens (patternfly#6030) * feat(hint): apply tokens * feat(hint): fix typo * feat(hint): update spacing * feat(hint): update border radius * feat(hint): fix heading font * feat(text, title, list): apply tokens (patternfly#6031) * feat(page): remove padding on nav sidebar (patternfly#6053) * fix(tokens): adjust font-weight base for headings (patternfly#6070) * feat(login): apply tokens (patternfly#6028) * feat(modal): add tokens to modal and about modal (patternfly#6042) * fix(font): use variable font, add opt-in for legacy font (patternfly#6073) * feat(ExpandableSection): added tokens (patternfly#6063) * feat(Tooltip): applied tokens (patternfly#6064) * feat(Popover): applied tokens (patternfly#6065) * feat(BackToTop): refactor with tokens (patternfly#6085) * feat(SkipToContent): refactor with tokens (patternfly#6090) * feat(Accordion): applied tokens (patternfly#6058) * feat(banner): refactor with tokens (patternfly#6081) * feat(InlineEdit): applied tokens Took a stab at applying the tokens here. There are some items that will be fixed with icon buttons. * chore(tokens): updates tokens from figma (patternfly#6103) * chore(tokens): updating usage of new font tokens (patternfly#6111) * feat(breadcrumb): apply tokens (patternfly#6060) * feat(HelperText): apply tokens (patternfly#6118) * feat(HelperText): first tokens * feat(HelperText): tokens changes * feat(HelperText): fixes * feat(Pagination): applied tokens (patternfly#6089) * feat(Pagination): applied tokens * Removed themes directory * Removed button and menutoggle styles, added wrapper * Removed padding vars for pagination container * Updated tokens and vars for padding * fix(Pagination): updated property order * Updated var names to remove md text * feat(avatar): apply tokens (patternfly#6122) * chore(ExpandableSection): updated Penta docs and focus/hover styling (patternfly#6124) * feat(EmptyState): apply tokens (patternfly#6056) * fix(form): keep form groups/controls from growing too wide (patternfly#6109) * chore(deps): update dependency @patternfly/react-core to v5.1.2 (patternfly#6113) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency @patternfly/react-code-editor to v5.1.2 (patternfly#6112) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency @patternfly/react-table to v5.1.2 (patternfly#6114) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * fix(card/menu-toggle/check): alignment bugs (patternfly#6110) * feat(SimpleList): pull in new v6 branch (patternfly#6128) * chore: turn on dryrun until release fixed [ci skip] --------- Co-authored-by: Sarah Rambacher <srambach@redhat.com> Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com> Co-authored-by: Matt Nolting <matthew.nolting@gmail.com> Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com> Co-authored-by: Austin Sullivan <ausulliv@redhat.com> Co-authored-by: Andrew Ronaldson <aronalds@redhat.com> Co-authored-by: kmcfaul <45077788+kmcfaul@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

patternfly-build · 2024-01-14T14:40:02Z

🎉 This PR is included in version 6.0.0-alpha.61 🎉

The release is available on:

Your semantic-release bot 📦🚀

srambach requested review from andrew-ronaldson, lboehling, mattnolting and mcoker November 8, 2023 14:53

mcoker approved these changes Nov 9, 2023

View reviewed changes

srambach linked an issue Nov 13, 2023 that may be closed by this pull request

Consume tokens in: Modal and About Modal #5707

Closed

srambach added 6 commits November 21, 2023 14:08

feat(modal): add example and documentation

672a9a6

feat(modal): add tokens to about modal

95717dd

feat(modal): new background image

7fe0cd8

feat(modal): clip rounded corners

84eece1

feat(modal): address comments

93e212a

feat(modal): fix spacing on descr and close btn

c3f8b27

srambach force-pushed the 5707-modal-tokens branch from 5505a44 to c3f8b27 Compare November 21, 2023 19:09

srambach requested a review from mcoker November 21, 2023 20:56

mcoker approved these changes Nov 21, 2023

View reviewed changes

feat(modal): remove comment

365cdb6

mcoker merged commit 8556435 into patternfly:v6 Nov 21, 2023

mcoker mentioned this pull request Nov 21, 2023

Consume tokens in: Modal and About Modal #5707

Closed

patternfly-build added the released on @alpha label Nov 21, 2023

mcoker pushed a commit to mcoker/patternfly that referenced this pull request Dec 1, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

39ea65e

nicolethoen pushed a commit that referenced this pull request Dec 12, 2023

feat(modal): add tokens to modal and about modal (#6042)

f8755d8

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 12, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

8ef7529

nicolethoen pushed a commit that referenced this pull request Dec 12, 2023

feat(modal): add tokens to modal and about modal (#6042)

6c259af

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 13, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

9ff9fe9

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 13, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

c9fbc45

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 19, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

28a1135

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 27, 2023

feat(modal): add tokens to modal and about modal (patternfly#6042)

cd1a46b

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 2, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

364f7d9

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 3, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

061a4aa

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 4, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

fd9fbd4

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

a81370b

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

a239864

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

cf37ca1

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

d695ad7

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

98ba2e5

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 15, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

96e0d52

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 16, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

995fc98

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 16, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

ac2630e

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 16, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

46f6201

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Jan 16, 2024

feat(modal): add tokens to modal and about modal (patternfly#6042)

e2188ca

srambach deleted the 5707-modal-tokens branch April 6, 2024 02:25

patternfly-build mentioned this pull request Apr 16, 2025

The automated release is failing 🚨 #7464

Closed

		--#{$about-modal-box}--BackgroundSize--width: 100%;
		--#{$about-modal-box}--BackgroundSize: contain;

feat(modal): add tokens to modal and about modal #6042

feat(modal): add tokens to modal and about modal #6042

Uh oh!

Conversation

srambach commented Nov 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patternfly-build commented Nov 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lboehling commented Nov 8, 2023

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

srambach Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

mcoker Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srambach Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

mcoker Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

mcoker Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

mcoker Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

srambach commented Nov 21, 2023

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Nov 21, 2023

Uh oh!

patternfly-build commented Jan 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

srambach commented Nov 8, 2023 •

edited

Loading

patternfly-build commented Nov 8, 2023 •

edited

Loading

mcoker Nov 9, 2023 •

edited

Loading