grafana / grafana/pyroscope

Simplify App Selector Modal

Open
#1,997 2 comments 0 reactions 1 assignee Claimed by @darrenjaneczek View on GitHub
frontend
Dominant language
Go
Stars
11.7k
Forks
802
Avg merge
1d 19h
Merged PRs (30d)
80

Description

In https://github.com/grafana/phlare/pull/858 we added a temporary fix where we make it so that the app selector modal size changes dynamically based on the content of the modal. For now this is more of a hotfix -- because the way that I fixed it I basically just copied the version from grafana/pyroscope and adapted it to the phlare app selector (which overrides the one from grafana/pyroscope)

In reality this method is prone to bugs because it has some slightly suspicious logic for calculating the height of the modal. @petethepig and @eh-am brought up a great point that this is the whole point of "max-height" and "auto" for overflow / scrolling etc...

We should simplify this logic to not require manually calculating the size of the modal and instead use max-height

simply doing this:
```
.appSelectorModal {
/* upstream code is positioned based on a label */
left: initial !important;
max-width: calc(100vw - 290px);
max-height: calc(100vh - 220px);
}
```

gets us close:

https://github.com/grafana/pyroscope/assets/23323466/0ce7eb4b-5902-4665-bf93-782407cb70ab

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.