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.

Sign up for GitHub

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

Jump to bottom

JsonGrid fixes #15346

Merged
merged 4 commits into from Feb 24, 2021
Merged

JsonGrid fixes #15346

merged 4 commits into from Feb 24, 2021

Conversation

Jako
Copy link
Collaborator

@Jako Jako commented Dec 4, 2020 •
edited

What does it do?

  • Change the record value of the currently edited field on keyup
  • Don't use a menu in the action column for deleting a record.
  • Remove the column menu for the grid.

Why is it needed?

  • When the user clicks on save before the currently edited field loses the focus, the changes in the current field are not saved.
  • The menu in the action column does not make sense, since there is only one delete entry in that menu.
  • The column menu does not make sense, since the columns are not sortable (the grid is sortable by drag & drop).

How to test

Create a new system setting with the field type JSON Grid and save that setting. The JSON grid is visible during editing the the setting again in the edit window at the bottom.

Related issue(s)/PR(s)

None known

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Dec 4, 2020
@Ibochkarev Ibochkarev added the pr/review-needed Pull request requires review and testing. label Dec 5, 2020
@Ibochkarev Ibochkarev added this to the v3.0.0-alpha3 milestone Feb 7, 2021
theboxer
theboxer requested changes Feb 11, 2021
View reviewed changes
Copy link
Member

@theboxer theboxer left a comment

Choose a reason for hiding this comment

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

I still see the context menu on the JsonGrid. I believe you'll have to remove the getMenu method to nuke it.

@Jako
Copy link
Collaborator Author

Jako commented Feb 13, 2021 •
edited

Thanks, the description of the fix was a bit wrong. I meant the menu in the action column, not the context menu.

@Ibochkarev Ibochkarev added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Feb 14, 2021
Jako and others added 4 commits February 24, 2021 12:06
@Jako @opengeek
Fix JsonGrid
ffba4ad 
- Change the gridEditor record value of the currently edited field on keyup
- Don't use the context menu. This does not make sense with only one entry in that menu
@Jako @opengeek
Fix a not displayed value change in the grid
2dd57d6
@Jako @opengeek
Remove the column menu for the grid and for the action columns.
8a8706b
@opengeek
grunt build
f1b7d73
@opengeek opengeek merged commit d615819 into modxcms:3.x Feb 24, 2021
@opengeek opengeek removed the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Feb 24, 2021
@Jako Jako deleted the fix-jsongrid branch February 26, 2021 11:01
@theboxer theboxer mentioned this pull request Jan 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theboxer theboxer theboxer approved these changes

@JoshuaLuckers JoshuaLuckers JoshuaLuckers approved these changes

@Mark-H Mark-H Awaiting requested review from Mark-H Mark-H is a code owner

@opengeek opengeek Awaiting requested review from opengeek opengeek is a code owner

Assignees
No one assigned
Labels
cla-signed CLA confirmed for contributors to this PR.
Projects
None yet
Milestone
v3.0.0-alpha3
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Jako @theboxer @JoshuaLuckers @opengeek @Ibochkarev