Skip to content

Conversation

falkoschindler
Copy link
Contributor

@falkoschindler falkoschindler commented Oct 14, 2025

Motivation

In #5276 we noticed that ui.aggrid's column auto-sizing break in the presence of flex columns.

Implementation

This PR conditionally ignores the auto_size_columns parameter if any column has a flex attribute.
This PR uses the "autoSizeStrategy" to implement the auto_size_columns parameter instead of calling a JavaScript function on size change.

Progress

  • I chose a meaningful title that completes the sentence: "If applied, this PR will..."
  • The implementation is complete.
  • Pytests are not necessary.
  • Documentation has been added.

@falkoschindler falkoschindler added this to the 3.1 milestone Oct 14, 2025
@falkoschindler falkoschindler requested a review from evnchn October 14, 2025 07:29
@falkoschindler falkoschindler added bug Type/scope: A problem that needs fixing review Status: PR is open and needs review labels Oct 14, 2025
Copy link
Collaborator

@evnchn evnchn left a comment

Choose a reason for hiding this comment

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

I can easily take this as a hotfix, as this code really fixes the non-display of AG Grid, and there's no valid use case of auto_size_columns and flex at the same time as of current NiceGUI version because it is now entirely broken.

Before:

image

After:

image

But I am still surprised why AG Grid broke this way, considering the docs have:

image

The docs do imply that sizeColumnsToFit and flex can be used together, and we may be not discovering the root issue with our hotfix approach.

@falkoschindler
Copy link
Contributor Author

@evnchn Maybe calling this.api.sizeColumnsToFit() isn't such a great idea. I just experimented with adding

'autoSizeStrategy': {'type': 'fitGridWidth'},

to the options dictionary and the result is flawless. It also solves the flickering from #5244.

@falkoschindler falkoschindler added in progress Status: Someone is working on it and removed review Status: PR is open and needs review labels Oct 14, 2025
@falkoschindler falkoschindler self-assigned this Oct 14, 2025
@evnchn
Copy link
Collaborator

evnchn commented Oct 14, 2025

Sure, maybe we can explore that.

But still, if the library's sane, no amount of reasonable tinkering with the options should result in a blank table, so I am calling it 🤡 in AG Grid.

@falkoschindler falkoschindler changed the title Don't auto-size columns if there are flex columns Fix auto-sizing of ui.aggrid Oct 14, 2025
@falkoschindler falkoschindler added review Status: PR is open and needs review and removed in progress Status: Someone is working on it labels Oct 14, 2025
@falkoschindler
Copy link
Contributor Author

I changed this PR to use the "autoSizeStrategy" - just to notice that this breaks when adding 'flex': 1 as in #5276. Not sure what I've done differently in #5277 (comment).

@falkoschindler falkoschindler added in progress Status: Someone is working on it and removed review Status: PR is open and needs review labels Oct 14, 2025
@falkoschindler
Copy link
Contributor Author

My current conclusion would be:

@evnchn
Copy link
Collaborator

evnchn commented Oct 15, 2025

I'm fine with this PR as it is, because we are at a state where if the config is invalid, console errors are produced.

Still though, no warnings, when doing sizeColumnsToFit on tables flex columns? 🤡


On a separate note, I'm considering to create a mechanism where browser warnings and errors are logged to the Python console. AG Grid will be a top user of this functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Type/scope: A problem that needs fixing in progress Status: Someone is working on it

Projects

None yet

2 participants