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

Page tree ajax request to fetch children #1149

Merged
merged 1 commit into from
Feb 10, 2022
Merged

Page tree ajax request to fetch children #1149

merged 1 commit into from
Feb 10, 2022

Conversation

melegiul
Copy link
Contributor

@melegiul melegiul commented Feb 7, 2022

Short description

Fetch page tree children by ajax request

Proposed changes

  • Page tree starts with root nodes only
  • On expanding new page children get queried and added to the DOM

Resolved issues

Fixes: #1101

@melegiul melegiul requested a review from a team as a code owner February 7, 2022 16:34
timobrembeck
timobrembeck reviewed Feb 8, 2022
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Amazing, thanks a lot! 💪 🚀 😍
I noticed a few problems, have a look at my comments below for details.

Apart from that, I think it would be cool to load the next children in advance after the page load and before the sub-pages are expanded, so that when the user clicks on expand, the results are already there... but this is probably out of scope for this PR, let's open another issue after this is finished...

integreat_cms/cms/templates/pages/page_tree_node.html Show resolved Hide resolved
integreat_cms/cms/views/pages/page_tree_view.py Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Show resolved Hide resolved
integreat_cms/cms/views/pages/page_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/pages/page_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/urls.py Outdated Show resolved Hide resolved
integreat_cms/cms/templates/pages/page_tree_node.html Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Show resolved Hide resolved
timobrembeck
timobrembeck reviewed Feb 8, 2022
View reviewed changes
integreat_cms/cms/constants/page_context.py Outdated Show resolved Hide resolved
integreat_cms/cms/templates/pages/page_tree_node.html Show resolved Hide resolved
integreat_cms/cms/views/pages/page_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/pages/page_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/pages/page_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/templates/pages/page_tree_node.html Outdated Show resolved Hide resolved
integreat_cms/cms/templates/pages/_page_tree_children.html Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Show resolved Hide resolved
timobrembeck
timobrembeck reviewed Feb 8, 2022
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck 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 your changes! Only a few details are missing now.
And could you add a small message to the changelog about this?

integreat_cms/cms/templates/messages.html Outdated Show resolved Hide resolved
@melegiul
Copy link
Contributor Author

melegiul commented Feb 9, 2022

@timoludwig Thanks for your hints, I have now added a changelog entry, moved the archived message box and switched to a class based view
But there still remain probelms at the archived page tree view and with page filtering
I will have a look at them

@melegiul melegiul force-pushed the feature/ajax-page-tree branch 2 times, most recently from a8fcefa to 4e417ed Compare February 9, 2022 17:48
@melegiul
Copy link
Contributor Author

melegiul commented Feb 9, 2022

dab25ce this should fix the broken archived page tree
Page filters seem to be fine too

timobrembeck
timobrembeck reviewed Feb 9, 2022
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! 🚀
Just a few tiny details, then this PR is ready to merge.
Could you rebase your branch to resolve the conflicts and clean up the git history a bit?

integreat_cms/cms/views/pages/partial_page_tree_view.py Outdated Show resolved Hide resolved
integreat_cms/locale/de/LC_MESSAGES/django.po Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/pages/collapse-subpages.ts Outdated Show resolved Hide resolved
@melegiul @timobrembeck
Page tree ajax request to fetch children
989bc38 
Applied on unfiltered and unarchived page tree view
Archived or filtered page tree view remain loading all pages at once like

Co-authored-by: Timo Ludwig <ludwig@integreat-app.de>
@melegiul
Copy link
Contributor Author

@timoludwig Rebasing and squashing is done, Thanks for sticking to this rabbit hole 🐰 😉

timobrembeck
timobrembeck approved these changes Feb 10, 2022
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Amazing, thanks! 🙏

@melegiul melegiul merged commit 3559695 into develop Feb 10, 2022
@melegiul melegiul deleted the feature/ajax-page-tree branch February 10, 2022 12:14
This was referenced Feb 10, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timobrembeck timobrembeck timobrembeck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use AJAX for the page tree view
2 participants
@melegiul @timobrembeck