add banner variant to Overlay by anjapatel · Pull Request #117 · glitchdotcom/shared-components

anjapatel · 2020-07-10T00:51:32Z

This PR adds a banner variant for Overlay. The goal is to create an Overlay option that looks close to the one in this design spec: https://www.figma.com/file/LLBMwcB37R4fQrn7kgZKUi/061820_PrivateApps_MI%2BKAC?node-id=34%3A0

This introduces new variants in block.js for this Overlay banner, specifically. I commented above the relevant banners to hopefully make this clearer.

How to Test

Go to overlay.js
Add variant={'banner'} to the Overlay, Title, and Info components
Add variant={'alignright'} to the Actions component
When you open an overlay, it should look like the overlay in the attached screenshot:

5. Test in dark mode!

Osmose

Nice work, thanks for the PR!

One difference I noted between the normal overlay in master and in this branch is that the title bar has a white background instead of grey; we should fix that so that existing overlays don't change appearance at all.

lib/block.js

Osmose · 2020-07-10T14:57:34Z

lib/block.js

+    color: var(--colors-primary);
+    background-color: var(--colors-background);
+  `,
+  alignright: css`


This variant also removes the background color and top border, so we should probably also name it "banner" since it's more than just alignment.

Is alignrightbanner okay? Can also do actionsbanner because it would only be used there. I'm thrown errors when I try to hyphenate and I'm not sure if camel case is the convention here.

ohhh how about actionsbanner and infobanner under sectionVariants ?

Osmose · 2020-07-10T14:58:37Z

lib/overlay.js

+      <p>
+        The Overlay component renders an accessible modal. See <a href="#Story_Blocks_for_Overlay_and_Popover_content">blocks to use with Overlays</a>
+        .
+      </p>


Can we add a second button to the story docs showing the variant overlay and how to use it?

lib/overlay.js

jlord · 2020-07-10T15:26:03Z

Will the dropshadow, buttons and 'X' be updated to match the design in Figma in this PR?

anjapatel · 2020-07-10T15:46:21Z

Will the dropshadow, buttons and 'X' be updated to match the design in Figma in this PR?

@jlord I missed the dropshadow on the Overlay, so I will pull that out in this PR! The "X" can just be deleted and the buttons can be styled independently, but I can make everything look like the Figma design in the example button I'm adding in now!

ackristen · 2020-07-10T15:53:38Z

@anjapatel this looks great! Just want to call out typography and spacing feedback:
(I'm not viewing this locally just referencing the screen shot so forgive me if it's already correct!)

Body copy: 14px
Heading: 16px
Spacing: 6px between heading and body and then 24px between body and buttons

jlord · 2020-07-10T17:02:10Z

@ackristen question on the radius of the modal: in Figma on its own it's shown with rounded corners on each corner but in the screenshot of it in the editor, there is no to top radius.

ackristen · 2020-07-10T17:06:45Z

@ackristen question on the radius of the modal: in Figma on its own it's shown with rounded corners on each corner but in the screenshot of it in the editor, there is no to top radius.

Originally:
In the editor, I imagined it sitting flush below the editor header.

Update:
I found another place to use this component within the private project experience so we can keep the radius on all 4 corners.

Sorry for the confusion!

anjapatel · 2020-07-10T17:12:30Z

@ackristen is this looking better?

Edit: I did put 6px of space between the header and body, but it does look a little more spaced out than the figma. I'm combing through now in case there's some extra padding somewhere. Let me know if this is on the right track!

ackristen · 2020-07-10T17:15:23Z

@ackristen is this looking better?

Edit: I did put 6px of space between the header and body, but it does look a little more spaced out than the figma. I'm combing through now in case there's some extra padding somewhere. Let me know if this is on the right track!

This looks good to me! I'm happy with it 👏

… sizes

Osmose

Looks good, nice work! Once the fix for useOverlay is in we can land this.

Osmose · 2020-07-10T17:34:37Z

lib/overlay.js

 export const useOverlay = () => {
-  const [open, setOpen] = React.useState(false);
+  const [openNormal, setOpenNormal] = React.useState(false);
+  const [openBanner, setOpenBanner] = React.useState(false);
  const toggleRef = React.useRef();
-  const onOpen = () => setOpen(true);
+  const onOpenNormal = () => setOpenNormal(true);
+  const onOpenBanner = () => setOpenBanner(true);
  const onClose = () => {
-    setOpen(false);
+    setOpenNormal(false);
+    setOpenBanner(false);
    toggleRef.current.focus();
  };
-  return { open, onOpen, onClose, toggleRef };
+  return { openNormal, onOpenNormal, openBanner, onOpenBanner, onClose, toggleRef };


useOverlay is actually exported for use outside shared-components, so we shouldn't add a second set of state values since it's meant to handle a single overlay. We can call it twice in StoryOverlay, though, once for each overlay.

anjapatel · 2020-07-10T18:23:54Z

Also @ackristen, this new overlay style is positioned against the top of the browser window. Is this okay? The overlay is positioned relative to the browser so it would take a lot more time for me to find a way to work around that.

If this doesn't work for what you have in mind for the future, let me know! Maybe I can add this style variant to a different component that's easier to position.

Osmose · 2020-07-10T20:54:14Z

Approved via Slack.

keithk · 2020-11-10T20:08:38Z

🚀 PR was released in v0.19.0 🚀

add banner variant to Overlay

792164a

anjapatel requested a review from Osmose July 10, 2020 00:51

Osmose suggested changes Jul 10, 2020

View reviewed changes

create example buttton for banner overlay

517563c

adjust names, margins, and add variant to TitleContent to adjust font…

b26f1d2

… sizes

Osmose approved these changes Jul 10, 2020

View reviewed changes

fix useOverlay

21c13e7

Osmose merged commit 94b502b into master Jul 10, 2020

Osmose deleted the anjali/add-variant-to-overlay branch July 10, 2020 20:54

keithk added the released label Nov 10, 2020

Conversation

anjapatel commented Jul 10, 2020

Uh oh!

Osmose left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Osmose Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

anjapatel Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

anjapatel Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

Osmose Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

anjapatel Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jlord commented Jul 10, 2020

Uh oh!

anjapatel commented Jul 10, 2020

Uh oh!

ackristen commented Jul 10, 2020

Uh oh!

jlord commented Jul 10, 2020

Uh oh!

ackristen commented Jul 10, 2020

Uh oh!

anjapatel commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ackristen commented Jul 10, 2020

Uh oh!

Osmose left a comment

Choose a reason for hiding this comment

Uh oh!

Osmose Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

anjapatel commented Jul 10, 2020

Uh oh!

Osmose commented Jul 10, 2020

Uh oh!

keithk commented Nov 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anjapatel commented Jul 10, 2020 •

edited

Loading