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
simonswine
commented
1 year ago
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