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

Grid: Remove deprecated usage of displayDensity #14015

Closed
simeonoff opened this issue Mar 25, 2024 · 1 comment · Fixed by #14163
Closed

Grid: Remove deprecated usage of displayDensity #14015

simeonoff opened this issue Mar 25, 2024 · 1 comment · Fixed by #14163
Assignees
Labels
display-density grid refactoring triage: blocking version: 18.0.x ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.

Comments

@simeonoff
Copy link
Collaborator

simeonoff commented Mar 25, 2024

The grid component relies on the value of the deprecated displayDensity token when calculating the heights of various elements in the grid. These values should rather come from the styles and be read by the grid's via computedStyles if they need to be factored in the sizing of the grid. The reason being is that the grid doesn't fully work with the --ig-size property right now (runtime) because it heavily depends on hardcoded values in the business logic.

@mddragnev
Copy link
Member

mddragnev commented Apr 4, 2024

I've been looking into this for a couple of days now and so far I have started to refactor the usage of the display density. There have been some issues like: some hardcoded values inside the grid for default width/height that are used broadly inside the for-of/pipes etc. so I have logged a separate refactoring task because I think that it would take a lot of time and effort to refactor and then test these things. The approach I follow now is to use a mutation observer to listen to changes to the style attribute of the grid which would give us the ability to react to runtime changes of the size css property which worked fine until today. The issue i came upon is as follows:
I dont think that there is any specification for that but I believe that each and every child component of the grid (for example ESF) could have --ig-sze style added to it that differs from the grid one. Then I found out that in order to achieve this so called "runtime" change of the size we need to again observe component when it changes the size style. A possible solution might be to let the mutation observer of the grid listen to its child tree style changes too but I think that this approach is bad because there would be too much invokations of the observer. Currently, the ESF sizing works partly: if we change the --ig-size variable from the devtools then some of the styles that the component is setting through typescript like min/max height does not update because we would need a new change detection. On the other hand, if we update the esf size by using a button that is not inside the grid programatically then the ESF closes and when you reopen it, everything seems good. I believe that this is not the whole use case. So this is why I am writing to you guys @dkamburov @simeonoff @damyanpetev @rkaraivanov to let you know and if possible to start a discussion about those things

@mddragnev mddragnev added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display-density grid refactoring triage: blocking version: 18.0.x ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants