Skip to content
This repository was archived by the owner on Sep 30, 2025. It is now read-only.

Conversation

aaronccasanova
Copy link
Member

@aaronccasanova aaronccasanova commented Nov 22, 2022

WHY are these changes introduced?

Resolves #7786.
Updates typography migration to insert comments for complex text migrations.

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

@laurkim laurkim added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Nov 23, 2022
@laurkim
Copy link
Contributor

laurkim commented Nov 23, 2022

Added the Skip changelog label temporarily so that other CI checks run.

@laurkim laurkim force-pushed the update-text-migration branch 3 times, most recently from 9c9ee88 to e5d9db9 Compare November 28, 2022 19:01
@laurkim laurkim force-pushed the update-text-migration branch from e5d9db9 to b8db06b Compare November 28, 2022 19:05
@aaronccasanova aaronccasanova self-assigned this Nov 29, 2022
@laurkim laurkim added #gsd:29805 and removed 🤖Skip Changelog Causes CI to ignore changelog update check. labels Nov 29, 2022
@aaronccasanova
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @aaronccasanova! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221129183721
yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20221129183721
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20221129183721
yarn add @shopify/polaris@0.0.0-snapshot-release-20221129183721

@laurkim laurkim changed the title WIP Update react-replace-text-components migration Update react-replace-text-components migration Dec 5, 2022
@laurkim laurkim marked this pull request as ready for review December 5, 2022 15:43
@laurkim laurkim requested review from sam-b-rose and laurkim December 5, 2022 15:44
extensions: 'tsx,ts,jsx,js',
parser: 'tsx',
verbose: 2,
runInBand: true,
Copy link
Member

Choose a reason for hiding this comment

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

nice!! ⚡

@laurkim
Copy link
Contributor

laurkim commented Dec 7, 2022

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

🫰✨ Thanks @laurkim! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221207182522
yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20221207182522
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20221207182522
yarn add @shopify/polaris@0.0.0-snapshot-release-20221207182522

@sam-b-rose
Copy link
Member

Is this ready to be merged into main? @aaronccasanova

@aaronccasanova
Copy link
Member Author

@samrose Thanks for the follow up. Lo mentioned before the holiday break that there are certain cases where legacy typography components are nested and replacements are causing styling/cascade issues. Before merging, I think we need to understand the issue further and decide if we can fix it programmatically or if we need to document a manual workflow.

@samrose
Copy link

samrose commented Jan 4, 2023

@samrose3 they were looking for you ^^^

@sam-b-rose
Copy link
Member

After trying out the snapshot of this migration on web, I noticed an edge-case we are missing with nested TextStyle components and injecting InlineCode:

<TextStyle>
  This is nested <TextStyle variation="code">InlineCode</TextStyle>
</TextStyle>

The above will not properly inject the InlineCode component. I've tried adjusting the migration script to account for this scenario but without luck. This is something we'll likely want to address or at least add a manual check comment.

@aaronccasanova
Copy link
Member Author

Good catch @samrose3! The issue was the removeJSXAttributes util was removing variation attributes deeply. Here's my WIP fix: 5783997

@sam-b-rose
Copy link
Member

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2023

🫰✨ Thanks @samrose3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20230107135043
yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20230107135043
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230107135043
yarn add @shopify/polaris@0.0.0-snapshot-release-20230107135043

@aaronccasanova aaronccasanova merged commit 56b0bc2 into main Jan 10, 2023
@aaronccasanova aaronccasanova deleted the update-text-migration branch January 10, 2023 19:50
kyledurand pushed a commit that referenced this pull request Jan 11, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris@10.19.0

### Minor Changes

- [#7817](#7817)
[`6c310101d`](6c31010)
Thanks [@henryyi](https://github.com/henryyi)! - Added
`secondaryActions` prop in Navigation.Item to support up to two actions

### Patch Changes

- [#8018](#8018)
[`2620b0a20`](2620b0a)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed modal
scroll bug

## @shopify/stylelint-polaris@5.1.0

### Minor Changes

- [#7993](#7993)
[`128f147d1`](128f147)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Created
`polaris/declaration-property-value-disallowed-list` rule to ignore
failures in `@font-face` at-rules

### Patch Changes

- [#8006](#8006)
[`6404b1638`](6404b16)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! -
Temporarily disabled `border-radius-base` error

## @shopify/plugin-polaris@0.0.26

### Patch Changes

- Updated dependencies
\[[`fad3809ef`](fad3809),
[`56b0bc2dc`](56b0bc2)]:
    -   @shopify/polaris-migrator@0.10.3

## @shopify/polaris-migrator@0.10.3

### Patch Changes

- [#7997](#7997)
[`fad3809ef`](fad3809)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
`styles-tokenize-font` to ignore `@font-face` at-rules


- [#7783](#7783)
[`56b0bc2dc`](56b0bc2)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Improve
robustness of the `react-replace-text-components` migration

- Updated dependencies
\[[`6404b1638`](6404b16),
[`128f147d1`](128f147)]:
    -   @shopify/stylelint-polaris@5.1.0

## polaris.shopify.com@0.28.3

### Patch Changes

- Updated dependencies
\[[`2620b0a20`](2620b0a),
[`6c310101d`](6c31010)]:
    -   @shopify/polaris@10.19.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@gwyneplaine gwyneplaine mentioned this pull request Feb 14, 2023
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
Co-authored-by: Lo Kim <lo.kim@shopify.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris@10.19.0

### Minor Changes

- [Shopify#7817](Shopify#7817)
[`6c310101d`](Shopify@6c31010)
Thanks [@henryyi](https://github.com/henryyi)! - Added
`secondaryActions` prop in Navigation.Item to support up to two actions

### Patch Changes

- [Shopify#8018](Shopify#8018)
[`2620b0a20`](Shopify@2620b0a)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed modal
scroll bug

## @shopify/stylelint-polaris@5.1.0

### Minor Changes

- [Shopify#7993](Shopify#7993)
[`128f147d1`](Shopify@128f147)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Created
`polaris/declaration-property-value-disallowed-list` rule to ignore
failures in `@font-face` at-rules

### Patch Changes

- [Shopify#8006](Shopify#8006)
[`6404b1638`](Shopify@6404b16)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! -
Temporarily disabled `border-radius-base` error

## @shopify/plugin-polaris@0.0.26

### Patch Changes

- Updated dependencies
\[[`fad3809ef`](Shopify@fad3809),
[`56b0bc2dc`](Shopify@56b0bc2)]:
    -   @shopify/polaris-migrator@0.10.3

## @shopify/polaris-migrator@0.10.3

### Patch Changes

- [Shopify#7997](Shopify#7997)
[`fad3809ef`](Shopify@fad3809)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
`styles-tokenize-font` to ignore `@font-face` at-rules


- [Shopify#7783](Shopify#7783)
[`56b0bc2dc`](Shopify@56b0bc2)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Improve
robustness of the `react-replace-text-components` migration

- Updated dependencies
\[[`6404b1638`](Shopify@6404b16),
[`128f147d1`](Shopify@128f147)]:
    -   @shopify/stylelint-polaris@5.1.0

## polaris.shopify.com@0.28.3

### Patch Changes

- Updated dependencies
\[[`2620b0a20`](Shopify@2620b0a),
[`6c310101d`](Shopify@6c31010)]:
    -   @shopify/polaris@10.19.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Typography foundations] Update migration to insert comments when not JSXIdentifiers/strings
4 participants