Skip to content

Conversation

@alexandre-castelain
Copy link
Contributor

Summary

This merge request introduces the Column Visibility Groups feature, which allows users to organize table columns into different "views" and switch between them using a dropdown in the UI as proposed in #86. This is particularly useful for tables with many columns, enabling a cleaner and more user-friendly display.

Check the documentation to know how this works

Implementation Notes & Points of Attention

I have done my best to follow the bundle's structure, but I would appreciate feedback on a few specific points:

  • src/DataTable.php:
    private ?string $requestedColumnVisibilityGroup = null;
    Is this the right place for this property? It is used to keep track of the column visibility group selected by the user.

  • src/DataTableBuilder.php (line 969):
    I use ColumnVisibilityGroupBuilder to directly create ColumnVisibilityGroup instances. I did not use a factory, as it did not seem necessary here. Is this approach acceptable?

  • src/Type/DataTableType.php (line 121):
    I pass the ColumnVisibilityGroups directly to the view, without creating a ColumnVisibilityGroupView. Do you think a dedicated view class is needed, or is this sufficient?

  • src/ColumnVisibilityGroup/ColumnVisibilityGroupBuilder.php (line 26):
    I use the translator, but currently do not allow the use of the translation_domain defined in the DataTableType, nor do I allow overriding the translation_domain or adding arguments. Should this be improved to support these features?

  • Testing:
    I have not yet created tests for this feature. I would appreciate advice on which aspects should be covered by tests and any recommendations for structuring them.

  • Documentation:
    The documentation could potentially be expanded further. Please let me know if there are areas that require more detail or clarification.

Request for Feedback

  • What do you think of the feature and its usefulness?
  • Are there improvements you would suggest for the implementation or API?
  • Do you see any issues with the current approach regarding extensibility, maintainability, or bundle conventions?
  • Should the translation and view handling be made more flexible or standardized?

Thank you for your review and feedback!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant