[LegacyFilters] Fixed button spacing on ConnectedFilterControl
#11317
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
yurm04
commented
Dec 11, 2023
|
|
||
| &:has(.MoreFiltersButtonContainer.onlyButtonVisible) { | ||
| + * { | ||
| margin-left: var(--p-space-200); |
There was a problem hiding this comment.
@alex-page addressing your comment here and setting the spacing on the sibling element
There was a problem hiding this comment.
I think :has is controversial with browser support 🤦♂️
There was a problem hiding this comment.
I thought we were cool with :has since we were using it for the se23 flag. Am I misremembering that?
There was a problem hiding this comment.
We wanted to use :where() with the se23 flag to flatten the specificity. Both are disallowed per our minimum Safari on iOS version of v13.
There was a problem hiding this comment.
gotchya 👍 back to the drawing board
| @@ -110,6 +116,8 @@ | |||
| } | |||
|
|
|||
| .MoreFiltersButtonContainer.onlyButtonVisible { | |||
| padding-left: 0; | |||
There was a problem hiding this comment.
@lgriffee I think this should address your feedback and make sure that these changes don't affect the spacing at larger breakpoints
alex-page
reviewed
Dec 11, 2023
...t/src/components/LegacyFilters/components/ConnectedFilterControl/ConnectedFilterControl.scss
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Page <[email protected]>
...t/src/components/LegacyFilters/components/ConnectedFilterControl/ConnectedFilterControl.scss
Outdated
Show resolved
Hide resolved
added 3 commits
December 11, 2023 16:39
|
The updated approach is working well 👍 Seems to be fixed on all stories except some disabled 👀 UPDATE: Perhaps storybook was being flaky earlier, but now I see the missing gap on all stories. Screen.Recording.2023-12-11.at.3.01.14.PM.mov |
|
@aaronccasanova thanks for catching that 👍 I think I have all sizes and stories accounted for now 🤞 Updated the PR description with a new |
Comment on lines
128
to
134
| @media #{$p-breakpoints-md-down} { | ||
| margin-left: var(--p-space-200); | ||
| } | ||
|
|
||
| @media #{$p-breakpoints-md-up} { | ||
| margin-left: 0; | ||
| } |
There was a problem hiding this comment.
Quick tophat and this update is functioning as expected 🙌
Only light suggestion I have is to rewrite this mobile first e.g.
.AuxiliaryContainer {
flex-grow: 0;
margin-left: var(--p-space-200);
@media #{$p-breakpoints-md-up} {
margin-left: 0;
}
}
Merged
alex-page
pushed a commit
that referenced
this pull request
Dec 15, 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/[email protected] ### Minor Changes - [#11349](#11349) [`0a2f1659d`](0a2f165) Thanks [@alex-page](https://github.com/alex-page)! - Add PageClock and PageClockFilled icons ## @shopify/[email protected] ### Minor Changes - [#11317](#11317) [`e197ca5f6`](e197ca5) Thanks [@yurm04](https://github.com/yurm04)! - [LegacyFilters] Fixed button spacing on `ConnectedFilterControl` ### Patch Changes - [#11340](#11340) [`3213735a0`](3213735) Thanks [@jesstelford](https://github.com/jesstelford)! - Internal refactor: CSS Module files are renamed from _.scss to _.module.scss - [#11269](#11269) [`bc4272a2e`](bc4272a) Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added an optional `hidden` property to the `Filters` `FilterInterface` type to support `filters` that are only set programmatically - Updated dependencies \[[`0a2f1659d`](0a2f165)]: - @shopify/[email protected] ## [email protected] ### Patch Changes - [#11269](#11269) [`bc4272a2e`](bc4272a) Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added an optional `hidden` property to the `Filters` `FilterInterface` type to support `filters` that are only set programmatically - Updated dependencies \[[`3213735a0`](3213735), [`0a2f1659d`](0a2f165), [`e197ca5f6`](e197ca5), [`bc4272a2e`](bc4272a)]: - @shopify/[email protected] - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba
pushed a commit
to AnnaCheba/polaris
that referenced
this pull request
Apr 22, 2024
…pify#11317) <!-- ☝️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 - Open it as a draft if it’s a work in progress --> ### WHY are these changes introduced? Fixes Shopify#10216 <!-- Context about the problem that’s being addressed. --> ### WHAT is this pull request doing? Before https://github.com/Shopify/polaris/assets/4642404/657acfd9-4b53-4860-999f-7527603eb357 After https://github.com/Shopify/polaris/assets/4642404/cfd07deb-7dc0-4435-95e4-91b06390dc96 <!-- Summary of the changes committed. Before / after screenshots are appreciated for UI changes. Make sure to include alt text that describes the screenshot. Include a video if your changes include interactive content. 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> --> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces) 🗒 [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) ### 🎩 checklist - [ ] Tested a [snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases) - [ ] 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: Alex Page <[email protected]>
ascherkus
pushed a commit
to ascherkus/polaris
that referenced
this pull request
Feb 19, 2025
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/[email protected] ### Minor Changes - [Shopify#11349](Shopify#11349) [`0a2f1659d`](Shopify@cb2558b) Thanks [@alex-page](https://github.com/alex-page)! - Add PageClock and PageClockFilled icons ## @shopify/[email protected] ### Minor Changes - [Shopify#11317](Shopify#11317) [`e197ca5f6`](Shopify@d9e20b7) Thanks [@yurm04](https://github.com/yurm04)! - [LegacyFilters] Fixed button spacing on `ConnectedFilterControl` ### Patch Changes - [Shopify#11340](Shopify#11340) [`3213735a0`](Shopify@47c2303) Thanks [@jesstelford](https://github.com/jesstelford)! - Internal refactor: CSS Module files are renamed from _.scss to _.module.scss - [Shopify#11269](Shopify#11269) [`bc4272a2e`](Shopify@6136eb3) Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added an optional `hidden` property to the `Filters` `FilterInterface` type to support `filters` that are only set programmatically - Updated dependencies \[[`0a2f1659d`](Shopify@cb2558b)]: - @shopify/[email protected] ## [email protected] ### Patch Changes - [Shopify#11269](Shopify#11269) [`bc4272a2e`](Shopify@6136eb3) Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added an optional `hidden` property to the `Filters` `FilterInterface` type to support `filters` that are only set programmatically - Updated dependencies \[[`3213735a0`](Shopify@47c2303), [`0a2f1659d`](Shopify@cb2558b), [`e197ca5f6`](Shopify@d9e20b7), [`bc4272a2e`](Shopify@6136eb3)]: - @shopify/[email protected] - @shopify/[email protected] 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes #10216
WHAT is this pull request doing?
Before
Screen.Recording.2023-12-11.at.3.59.21.PM.mov
After
Screen.Recording.2023-12-11.at.3.58.03.PM.mov
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.mdwith documentation changes