Simplify App Selector Modal
- 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.