Skip to content
This repository has been archived by the owner on Dec 21, 2021. It is now read-only.

AUI-3206 append container as we expand tree item #176

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

NemethNorbert
Copy link
Member

Hey,

AUI-3206,
LPS-140686 - LPS to commit the solution to liferay

The tree view was not able to expand if we loaded the items as collapsed after https://issues.liferay.com/browse/AUI-3205
Please review my change.

Thanks,

@georgel-pop-lr
Copy link
Member

Hey,

AUI-3206, LPS-140686 - LPS to commit the solution to liferay

The tree view was not able to expand if we loaded the items as collapsed after https://issues.liferay.com/browse/AUI-3205 Please review my change.

Thanks,

I have tested this and seems LGTM from my point of view.

@NemethNorbert
Copy link
Member Author

Hey, can someone provide an update on this PR?

Thanks,

Copy link
Member

@markocikos markocikos left a comment

Choose a reason for hiding this comment

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

@NemethNorbert See a minor tweak inline.

This 'load more' behavior of setting container as internal value, but not appending and expanding immediately, is quite weird. I am not sure if it is intended behavior or a bug. But, I quite like the idea of not expanding and rendering nodes immediately. If we are already hitting 'load more', we are dealing with a lot of pages. I know some customers have tens of thousands of pages. So, if it's a bug, the bug gets 👍 from me 😄

Comment on lines 722 to 724
if (nodeContainer) {
boundingBox.append(nodeContainer);
}
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a check if the node is already appended? Something like

if (nodeContainer && !boundingBox.contains(nodeContainer)) {

That way we would revert to standard component behavior of show/hide after the first append.

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

Successfully merging this pull request may close these issues.

3 participants