Skip to content

Conversation

@yurm04
Copy link
Contributor

@yurm04 yurm04 commented May 25, 2023

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-summer-editions/issues/23

WHAT is this pull request doing?

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

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

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@yurm04 yurm04 changed the base branch from main to v11.x.x May 25, 2023 19:45
@github-actions github-actions bot added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label May 25, 2023
@yurm04 yurm04 mentioned this pull request May 25, 2023
5 tasks
@github-actions github-actions bot removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label May 25, 2023
@yurm04 yurm04 marked this pull request as ready for review May 26, 2023 16:25
@yurm04 yurm04 requested review from sarahill and sophschneider May 26, 2023 18:33
Copy link
Contributor

@sophschneider sophschneider left a comment

Choose a reason for hiding this comment

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

🙌 LGTM! just left a comment about the font weight and custom border radius.

@sophschneider sophschneider self-requested a review May 30, 2023 16:57
@sophschneider sophschneider force-pushed the ye/badge branch 2 times, most recently from 83c9795 to c115608 Compare May 30, 2023 21:24
@sophschneider sophschneider force-pushed the ye/badge branch 2 times, most recently from ea72628 to 07f91d8 Compare May 31, 2023 19:39
value: colors.red[200],
valueExperimental: colorsExperimental.red[4],
valueExperimental: colorsExperimental.red[6],
description: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

@sophschneider
Copy link
Contributor

@kyledurand @johanstromqvist Badge is ready for review! You can check out the storybook here.

before after design
Screenshot 2023-05-31 at 3 40 42 PM Screenshot 2023-05-31 at 3 40 48 PM Screenshot 2023-05-31 at 3 42 33 PM

I put all the badges in one story for easy tophatting, I'll revert it after review. There is also an Icon variant that we haven't discussed, but its only used once in the Admin so I don't think its worth coming up for designs/adding logic for summer editions. I created an issue for after editions here.

);
}

export function DesignReview() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I will revert this mega story after review

@johanstromqvist
Copy link
Contributor

johanstromqvist commented May 31, 2023

@kyledurand @johanstromqvist Badge is ready for review! You can check out the storybook here.

I put all the badges in one story for easy tophatting, I'll revert it after review. There is also an Icon variant that we haven't discussed, but its only used once in the Admin so I don't think its worth coming up for designs/adding logic for summer editions. I created an issue for after editions here.

Beautiful! Love the story and the backlog issue.

Re pip

  • What's your take on the pip being css or svg?
  • Did you test it one low res screen? I currently don't have access to one.
  • Is there a follow up action that needs to be captured in the post SE23 issue?

Copy link
Contributor

@johanstromqvist johanstromqvist left a comment

Choose a reason for hiding this comment

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

💯

@johanstromqvist
Copy link
Contributor

Contrast seemed low, but it passes. I guess merchants typically are scanning for non-fullfilled statuses on the screen anyways.

image image

Quick test using --p-color-text (left) instead of --p-color-text-subdued (right)

@sophschneider
Copy link
Contributor

sophschneider commented May 31, 2023

@johanstromqvist

Did you test it one low res screen? I currently don't have access to one.

No, since im only modifying the border radius and thickness on the pip it should be the same as the old pip. I do have browserstack though if you want to test the whole v11.x.x branch when we do a polish review

What's your take on the pip being css or svg?

I would personally rather use svgs but this is the most minimal change for summer editions. In the follow up PR after editions we can evaluate

Contrast seemed low, but it passes. I guess merchants typically are scanning for non-fullfilled statuses on the screen anyways.

I have similar concerns since subdued text on subdued bg is cutting it really close, but it seems to be a consistent pattern across designs.

Is there a follow up action that needs to be captured in the post SE23 issue?

wdym?

btw im merging this to move fast but feel free to respond on this PR or the original/follow up badge issue

@sophschneider sophschneider merged commit 62831cb into v11.x.x May 31, 2023
@sophschneider sophschneider deleted the ye/badge branch May 31, 2023 22:39
sophschneider added a commit that referenced this pull request Jun 1, 2023
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes Shopify/archive-polaris-backlog-2024#23

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Sophie Schneider <thesophieschneider@gmail.com>
sophschneider added a commit that referenced this pull request Jun 1, 2023
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes Shopify/archive-polaris-backlog-2024#23

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Sophie Schneider <thesophieschneider@gmail.com>
@sophschneider sophschneider self-assigned this Jun 2, 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.

4 participants