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

[DEV-13410] Fix - Coverage placeholder search input #573

Conversation

kudlajz
Copy link
Contributor

@kudlajz kudlajz commented Nov 1, 2024

I've noticed some problems in the app with the usage of Popper, so I've brought back the minHeight and maxHeight calculation, so Popper doesn't have to flip the entire dropdown unless the available space is less than minHeight.

This makes the dropdown still usable while opened near the bottom of the screen.

There was also a bug with the position of the dropdown when addon is present - this was due to usage of bottom placement which anchors to the center of the element (the search input). I've changed it to anchor to bottom-end, so it's always aligned.

Another thing that has been fixed is the active element is now properly visible when the dropdown is displayed (previously the active item was sometimes hidden, since the dropdown was scrolled down a bit).

Screenshot 2024-11-01 at 15 53 04
Screenshot 2024-11-01 at 15 53 10
Screenshot 2024-11-01 at 15 53 33

@kudlajz kudlajz self-assigned this Nov 1, 2024
Copy link
Contributor

@mohammadxali mohammadxali left a comment

Choose a reason for hiding this comment

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

👍

if (container.current) {
const viewport = document.body.getBoundingClientRect();
const rect = container.current.getBoundingClientRect();
setMaxHeight(clamp(viewport.height - rect.top - 4, minHeight, maxHeight));
Copy link
Contributor

Choose a reason for hiding this comment

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

What is 4 magic number? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure but I think it's probably a bottom padding. I restored this code from how it was before.
I can add a constant for it, but I'm gonna rework these dialogues this week during my project so probably not worth spending extra time on this.

@kudlajz kudlajz merged commit c4f56f3 into main Nov 4, 2024
9 checks passed
@kudlajz kudlajz deleted the feature/dev-13410-pitch-aside-clicking-log-coverage-from-coverage-placeholder branch November 4, 2024 13:12
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