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

Settings grid adjustments for Extras support #16414

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Apr 4, 2023

What does it do?

Re-introduced original id property on settings grid filter combos and added a couple small changes already developed for the much larger #16369.

Why is it needed?

Provides a quick solution to a couple problems caused by the unintentional early implementation of #16089 into the 3.0.3 release, namely those related to Extras that extend the settings grid (specifically @Jako's Agenda and CrossContextsSettings).

How to test

  • Ensure that the ACL grids in the Usergroup Permissions area (5 grids) and the Settings grid behave as they currently do in 3.0.3.
  • Test the two Extras mentioned above to verify they behave as intended.

Related issue(s)/PR(s)

Fulfills request made in the closed #16408

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Apr 4, 2023
@smg6511 smg6511 added the requires build Grunt build is required for integration label Apr 4, 2023
@JoshuaLuckers JoshuaLuckers added this to the v3.0.4 milestone Apr 8, 2023
Copy link
Collaborator

@Jako Jako left a comment

Choose a reason for hiding this comment

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

Works well. Is it possible to deactivate the tab handling by option in an extended grid?

@smg6511
Copy link
Collaborator Author

smg6511 commented Apr 9, 2023

Is it possible to deactivate the tab handling by option in an extended grid?

I suppose that could be done, I'll have to give it a look. Note that the only things happening with the tab handling are:

  • when a filter is applied, the tab index is added to the URL for later retrieval,
  • when a URL with a tab parameter is directly accessed, the tab specified will be activated (currently only for panels having a grid), and
  • the filters for the grid within a panel being moved away from are always cleared.

Is this undesirable in your case?

@Jako
Copy link
Collaborator

Jako commented Apr 13, 2023

Currently I am stumbling over the following problem: I save and retrieve the tabs in my extras with a stateful property and not with a url property. So when using the system settings grid and changing the area there for example, the url gets the tab property. If I switch to another tab and reload the manager, the system settings tab is opened. This is because the url property takes precedence over the stateful property and my extras do not reset the tab url property when I switch tabs.

@smg6511
Copy link
Collaborator Author

smg6511 commented May 11, 2023

@Jako - Let's do this: I'll create a way for custom (i.e., Extras) grid classes to opt out of the system's tab/grid filter tracking; but I'll do it in a separate PR since it solves a different issue and might be considered more of a feature than a fix.

@smg6511 smg6511 added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Aug 16, 2023
@opengeek opengeek force-pushed the 3.x-settings-grid-filters-patch branch from 0f2e71d to 2770a9e Compare August 29, 2023 20:10
@opengeek opengeek merged commit 8feb62f into modxcms:3.x Aug 29, 2023
4 checks passed
opengeek added a commit that referenced this pull request Aug 29, 2023
Squashed commit of the following:

commit 9121214b1d1f1c093de9779fafb2004497a38a28
Author: Jason Coward <jason@opengeek.com>
Date:   Tue Aug 29 14:13:36 2023 -0600

    grunt build

commit 3a32455b58247735ab67adf362eeb2bebfbc8122
Author: Jim Graham <info@sparkmediagroup.com>
Date:   Tue Apr 4 16:25:45 2023 -0400

    Settings grid adjustments for Extras support
@smg6511 smg6511 deleted the 3.x-settings-grid-filters-patch branch March 27, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging. requires build Grunt build is required for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants