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

IBX-2814: Returned null instead of empty array when there are no child nodes #225

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Apr 27, 2022

Question Answer
JIRA issue https://issues.ibexa.co/browse/IBX-2814
Bug/Improvement yes
New feature no
Target version latest stable for bug fixes, master for new features
BC breaks yes
Tests pass yes
Doc needed yes (this should probably be mentioned somewhere)

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@ViniTou ViniTou force-pushed the ibx-2814-null-instead-of-empty-array branch from 6c9b75e to 8dcb3d5 Compare April 27, 2022 14:28
@ViniTou ViniTou requested a review from a team April 27, 2022 14:29
@dew326
Copy link
Member

dew326 commented Apr 28, 2022

Shouldn't we change checks in our custom tags twig templates? I know this is not a case in this version because AlloyEditor works differently than CKEditor but I think we should have twigs prepared for null.

Copy link
Member

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

Hence this PR is targetting ezplatform-richtext I assume version 3.3 is also affected - worth mentioning in the ticket.

@ViniTou
Copy link
Contributor Author

ViniTou commented Apr 28, 2022

@dew326
It works fine with current state of templates, as it simply adds empty html attribute.

@konradoboza @micszo
This issue is not reproducible on 3.3 as, Darek mentioned, AlloyEditor skips empty keys. But since it is technically possible to replace AE with CKEditor, and overall that solution helps us avoiding strange situations generally, I fixed it at 3.3.
So at 3.3, all you need to check if that it didnt broke anything.

@micszo micszo self-assigned this Apr 28, 2022
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Retested on Ibexa DXP v3.3 & v4.

@micszo micszo removed their assignment Apr 28, 2022
@ViniTou ViniTou merged commit 6510608 into 2.3 Apr 28, 2022
@ViniTou ViniTou deleted the ibx-2814-null-instead-of-empty-array branch April 28, 2022 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants