Skip to content

Conversation

@afairbrother
Copy link
Contributor

@afairbrother afairbrother commented Feb 2, 2022

WHY are these changes introduced?

Fixes unexpected text-wrap when focusing the secondaryAction in Firefox (referenced here: #4997 (review))

WHAT is this pull request doing?

Before:

152069988-cf302ba9-f7e8-4475-b97f-a483b17be159.mp4

After:

Screen.Recording.2022-02-02.at.10.22.47.mov

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';

import {Page, Banner} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
      <Banner
        title="Some of your product variants are missing weights"
        status="warning"
        action={{content: 'Edit variant weights', url: ''}}
        secondaryAction={{content: 'Learn more', url: ''}}
        onDismiss={() => {}}
      >
        <p>
          Add weights to show accurate rates at checkout and when buying shipping
          labels in Shopify.
        </p>
      </Banner>
    </Page>
  );
}

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2022

size-limit report

Path Size
cjs 164.36 KB (0%)
esm 94.28 KB (0%)
esnext 141.38 KB (+0.01% 🔺)
css 34.68 KB (+0.01% 🔺)

Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@lgriffee lgriffee left a comment

Choose a reason for hiding this comment

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

🚢✨🎉🔥 Thanks so much @afairbrother!

Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

Thanks again @afairbrother and nice catch @lgriffee!! 🚀

@afairbrother afairbrother force-pushed the fixSecondaryActionFocusInBanner branch from b13aed4 to 6e1b71a Compare February 2, 2022 18:13
@afairbrother afairbrother requested a review from dleroux February 2, 2022 19:56
@afairbrother
Copy link
Contributor Author

afairbrother commented Feb 2, 2022

Updated, and moved display: block to only be on focus. Thoughts on this, @dleroux ?

Before:
image

After:
Screen Shot 2022-02-02 at 14 54 45

@afairbrother afairbrother force-pushed the fixSecondaryActionFocusInBanner branch from 2db732c to ba8ea31 Compare February 2, 2022 19:59
@afairbrother afairbrother merged commit 1be078f into main Feb 3, 2022
@afairbrother afairbrother deleted the fixSecondaryActionFocusInBanner branch February 3, 2022 13:57
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.

4 participants