8000 Replacing toolbar contents and attributes intead of the whole toolbar… by nichoski · Pull Request #7086 · django-cms/django-cms · GitHub
[go: up one dir, main page]

Skip to content

Replacing toolbar contents and attributes intead of the whole toolbar… #7086

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

Conversation

nichoski
Copy link
Contributor
@nichoski nichoski commented Jul 1, 2021

Description

When a page is in non legacy structure mode, clicking the toolbar switcher button in order to show the cms structure board (sidebar), reloads the contents of the toolbar before making the structure board (sidebar) visible. And since our recent change in PR #7070, when contents of the toolbar are reloaded, the old toolbar element is replaced with the newly fetched one. However, the cms structure board does not update its reference to the toolbar object in this situation, and it ends up referencing the old, deleted toolbar. This causes a problem when the structure board tries to calculate the css style 'right' attribute, as it needs to account for the width of the toolbar object, and it ends up showing the toolbar outside the viewport. We didn't catch this when preparing PR #7070 because all of the pages we tested with initially were in legacy structure mode. And updating the toolbar after a plugin edit did not bring this issue, because the structure board was updated too in those cases.

Fix

Instead of replacing the old toolbar object with the new, we are now replacing only its contents and attributes. That way all the parts of the code that referenced the old toolbar object are still referencing the correct toolbar object.

Checklist

  • I have opened this pull request against develop
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on Slack to find a “pr review buddy” who is going to review my pull request.

@nichoski nichoski force-pushed the fix-sidebar-not-showing-in-non-legacy-mode-pages branch from e8114c9 to 7284d09 Compare July 1, 2021 23:52
@nichoski nichoski changed the base branch from develop to release/3.9.x July 2, 2021 10:05
@nichoski nichoski changed the base branch from release/3.9.x to develop July 2, 2021 12:52
@NicolaiRidani NicolaiRidani requested a review from a team October 8, 2021 09:35
@stale
Copy link
stale bot commented Jun 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 30, 2022
@marksweb
Copy link
Member
marksweb commented Jul 4, 2022

Closing this as I believe it's been done via another PR.

@marksweb marksweb closed this Jul 4, 2022
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.

2 participants
0