Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Top Bar Feature] Remove all references to dynamicTopBarAndReframe feature #12359

Merged
merged 5 commits into from
Jul 16, 2024

Conversation

mrcthms
Copy link
Contributor

@mrcthms mrcthms commented Jul 10, 2024

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-internal/issues/481

As part of the clean up around the dynamicTopBarAndReframe cleanup, we want to revert all updates for this feature within @shopify/polaris back to its initial state.

WHAT is this pull request doing?

Remove all references to code changes around the dynamicTopBarAndReframe and ensure behaviour is reverted back to how it was before the feature was introduced.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Flow sandbox with snapshot: https://flow.polaris-top-bar-test.marc-thomas.eu.spin.dev/dev-sandbox/overview/0190a740-3aad-7a63-a48b-30c3578f9a83

🎩 checklist

@mrcthms mrcthms force-pushed the mrcthms-remove-top-bar-beta-flag-code branch from d67efe2 to 6d62d85 Compare July 12, 2024 14:03
@mrcthms
Copy link
Contributor Author

mrcthms commented Jul 12, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @mrcthms! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240712141617"

@mrcthms mrcthms marked this pull request as ready for review July 12, 2024 14:52
@mrcthms
Copy link
Contributor Author

mrcthms commented Jul 12, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @mrcthms! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240712161118"

@@ -404,17 +348,6 @@ class FrameInner extends PureComponent<CombinedProps, State> {
);
};

private setBodyStyles = () => {
Copy link
Contributor

@laurkim laurkim Jul 12, 2024

Choose a reason for hiding this comment

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

I think this function is still needed for the conditionals that aren't checking for dynamicTopBarAndReframe, i.e., lines 408-410 and 414.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@laurkim I can't find reference to setBodyStyles in the Frame file, was it there or somewhere elsewhere you were seeing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no you're right, I just looked at the PR where the function was being added and looks like it's safe to remove 👍

@mrcthms mrcthms merged commit 8f0f1b7 into main Jul 16, 2024
13 checks passed
@mrcthms mrcthms deleted the mrcthms-remove-top-bar-beta-flag-code branch July 16, 2024 18:14
alex-page pushed a commit that referenced this pull request Jul 22, 2024
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-icons@9.3.0

### Minor Changes

- [#12233](#12233)
[`491cf8038`](491cf80)
Thanks [@ngkay](https://github.com/ngkay)! - Adds new icons for Shopify
Finance

## @shopify/polaris@13.8.0

### Minor Changes

- [#12359](#12359)
[`8f0f1b7e0`](8f0f1b7)
Thanks [@mrcthms](https://github.com/mrcthms)! - Removed all references
to the dynamicTopBarAndReframe feature and revert functionality back to
how it was

### Patch Changes

- Updated dependencies
\[[`491cf8038`](491cf80)]:
    -   @shopify/polaris-icons@9.3.0

## polaris.shopify.com@1.0.11

### Patch Changes

- Updated dependencies
\[[`8f0f1b7e0`](8f0f1b7),
[`491cf8038`](491cf80)]:
    -   @shopify/polaris@13.8.0
    -   @shopify/polaris-icons@9.3.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

2 participants