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

Make fuzzy finder modal wide again #41736

Merged
merged 2 commits into from
Sep 28, 2022
Merged

Make fuzzy finder modal wide again #41736

merged 2 commits into from
Sep 28, 2022

Conversation

olafurpg
Copy link
Member

@olafurpg olafurpg commented Sep 19, 2022

The width of the fuzzy finder modal recently became very narrow. I wasn't able to track down the exact PR that impacted the width, but at some point the priority of the Wildcard Modal component overwrote the custom width: 80vw style we defined specifically for the fuzzy finder. This commit fixes the priority by use an id attribute instead of class.

Before
CleanShot 2022-09-19 at 15 30 39

After
CleanShot 2022-09-19 at 15 29 46

Test plan

Manually tested the PR locally and posted screenshots in the PR description.

App preview:

Check out the client app preview documentation to learn more.

The width of the fuzzy finder modal recently became very narrow. I
wasn't able to track down the exact PR that impacted the width, but at
some point the priority of the Wildcard `Modal` component overwrote
the custom `width: 80vw` style we defined specifically for the fuzzy
finder. This commit fixes the priority by use an `id` attribute instead
of `class`.
@olafurpg
Copy link
Member Author

Added the 4.0 label since this is a bugfix, not a new feature.

@olafurpg
Copy link
Member Author

3:1 ✖ Expected "#modal" to have no more than 0 ID selectors selector-max-id

Any advice on how to fix the priority without using id?

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

It should not be happening — the Wildcard styles should have lower priority than component styles. This bug surfaces the fundamental issue with CSS rules order in the production environment that is different from what we have in the development env.

Let me look into the root cause. If the fix takes a lot of time, we can fix this side-effect to get it into the 4.0 release.

auto-merge was automatically disabled September 21, 2022 12:44

Merge could not be authorized

@olafurpg
Copy link
Member Author

@valerybugakov have you been able to investigate the root cause of this issue?

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Hey @olafurpg, sorry for the delay!

Here's the Slack thread if you're interested in more context and the tracking issue to address the underlying problem. I pushed the update to this PR that patches the side-effect.

@olafurpg
Copy link
Member Author

Thank you @valerybugakov ! 🙏🏻

@olafurpg olafurpg merged commit ad2118b into main Sep 28, 2022
@olafurpg olafurpg deleted the olafurpg/wider-fuzzy-finder branch September 28, 2022 07:46
sashaostrikov pushed a commit that referenced this pull request Sep 29, 2022
* Make fuzzy finder modal wide again

The width of the fuzzy finder modal recently became very narrow. I
wasn't able to track down the exact PR that impacted the width, but at
some point the priority of the Wildcard `Modal` component overwrote
the custom `width: 80vw` style we defined specifically for the fuzzy
finder. This commit fixes the priority by use an `id` attribute instead
of `class`.

* web: make fuzzy finder modal wide again

Co-authored-by: Valery Bugakov <skymk1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants