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

Fix sorting in page for translations #526

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

d-g-codappix
Copy link

@d-g-codappix d-g-codappix commented Aug 27, 2024

current behavior:

If pages are translated in the so-called “Free Mode”, then only the sort order in the default language is fixed. The sorting in translated pages is not changed.

expected behavior:
In “Free Mode”, however, the contents are independent of each other and should therefore also be fixed for all languages including the default language.

$container->getChildRecords() only used for the condition. We can remove
this, because the new getAfterContainerRecord() returns the container
itself if ce has no children. We than can check if the uids are equal.
This improvement allows to extract the $sorting from the condition so we
only have the else block.
@d-g-codappix d-g-codappix force-pushed the fix_sorting_in_page_for_translations branch from 376be64 to 8539644 Compare August 27, 2024 08:53
@achimfritz
Copy link
Contributor

Hi @d-g-codappix , thanks for PR, your CSV-Fixtures setups are not valid

for Tests/Functional/Integrity/Fixtures/SortingInPage/container_is_sorted_before_child_of_previous_container_with_nested_changed_children_sorting.csv

  • please speficy valid CTypes for all content-elements.

for Tests/Functional/Integrity/Fixtures/SortingInPage/container_is_sorted_before_child_of_previous_container_on_translated_page.csv

  • pid of all pages should be 0
  • translated pages should have l10n_parent=1
  • pid of all content elements should be 1
  • set CType for all content-elements

both files:

  • please set sorting for all content-elements
  • it would be nice (for me, to test), if all records have speaking title/header

@achimfritz
Copy link
Contributor

additional, i would suggest to drop the language parameter, but autodetect all pages in "Free Mode" (and also run only on these (no connected or mixed mode pages should be considered)

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

Successfully merging this pull request may close these issues.

2 participants