Skip to content

Fix: extend reload_records to support grid view #3780

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

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

m7madmagdy
Copy link

@m7madmagdy m7madmagdy commented Apr 4, 2025

Description

Fixes #3718

Support reload_records with grid and map views after running an action.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Screencast.from.04-06-2025.09.20.49.PM.webm

Manual review steps

  1. Step 1
  2. Step 2

Manual reviewer: please leave a comment with output from the test if that's the case.

@github-actions github-actions bot added the Fix label Apr 4, 2025
Copy link

codeclimate bot commented Apr 4, 2025

Code Climate has analyzed commit 9da6753 and detected 0 issues on this pull request.

View more on Code Climate.

@m7madmagdy m7madmagdy changed the title send view_type to reload_records method Fix: extend reload_records to support grid view Apr 4, 2025
@m7madmagdy m7madmagdy marked this pull request as ready for review April 6, 2025 19:29
@m7madmagdy m7madmagdy requested a review from adrianthedev April 6, 2025 19:30
Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this @m7madmagdy

The view type should be inferred by Avo.

@m7madmagdy m7madmagdy requested a review from Paul-Bob April 8, 2025 11:42
@m7madmagdy m7madmagdy marked this pull request as draft April 8, 2025 11:45
Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

Hi @m7madmagdy, I noticed that you requested a review and then marked the PR as a draft.

Feel free to ping me once it's ready for a full review.

@m7madmagdy m7madmagdy marked this pull request as ready for review April 23, 2025 18:56
@m7madmagdy m7madmagdy requested a review from Paul-Bob April 23, 2025 19:13
m7madmagdy and others added 4 commits April 25, 2025 15:37
This refactor makes the view type accessible within the resource instead of being computed and stored on @index_params
@Paul-Bob Paul-Bob changed the base branch from main to refactor/view_type_access April 28, 2025 16:34
@Paul-Bob Paul-Bob changed the base branch from refactor/view_type_access to main April 28, 2025 16:36
@Paul-Bob Paul-Bob changed the base branch from main to refactor/view_type_access April 28, 2025 16:38
@Paul-Bob Paul-Bob deleted the branch avo-hq:main April 28, 2025 17:46
@Paul-Bob Paul-Bob closed this Apr 28, 2025
@Paul-Bob Paul-Bob reopened this Apr 28, 2025
@Paul-Bob Paul-Bob changed the base branch from refactor/view_type_access to main April 28, 2025 17:48
Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

Hi @m7madmagdy, thanks for looking into this!

We're getting close to merging it. I've made this refactor that moves view_type from @index_params into the resource object. This way, we don't need to pass view_type through all the components like before. Also, for people using link_arguments, there is no need to manually include view_type as an argument anymore.

I also separated the reload row and reload grid functions. The grid item reload is much simpler and doesn't need to compute header fields or run other "heavy" logic.

Let me know what you think about these changes! Also, could you please add a test for this feature and a short paragraph in the docs mentioning that this feature is available for grid items starting from Avo 3.next_minor_version?

@m7madmagdy
Copy link
Author

Hi @Paul-Bob

This change looks good and much cleaner
I will add a test for this feature and update the documentation with a short paragraph mentioning that this feature is available for grid items

Thanks for the great refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reload_records not working with the Grid view
4 participants