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

EZP-32248: Retain content tree state #1679

Merged
merged 3 commits into from
Dec 23, 2020

Conversation

GrabowskiM
Copy link
Contributor

Question Answer
Tickets https://issues.ibexa.co/browse/EZP-32248
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

Checklist:

  • Ready for Code Review

@@ -26,10 +28,46 @@ export default class ContentTree extends Component {
this.clearDocumentResizingListeners();
}

componentDidMount() {
this.containerSrollRef.addEventListener('scroll', (event) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.containerSrollRef.addEventListener('scroll', (event) => {
this.containerScrollRef.addEventListener('scroll', (event) => {

@@ -26,10 +28,46 @@ export default class ContentTree extends Component {
this.clearDocumentResizingListeners();
}

componentDidMount() {
this.containerSrollRef.addEventListener('scroll', (event) => {
clearTimeout(this.scrollTimeout);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
clearTimeout(this.scrollTimeout);
window.clearTimeout(this.scrollTimeout);

To show that this is from window object

this.containerSrollRef.addEventListener('scroll', (event) => {
clearTimeout(this.scrollTimeout);

this.scrollTimeout = setTimeout(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.scrollTimeout = setTimeout(
this.scrollTimeout = window.setTimeout(

document.body.dispatchEvent(new CustomEvent('ez-content-tree-resized'));
}

if (this.props.items.length && !this.scrollPositionSet) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (this.props.items.length && !this.scrollPositionSet) {
if (this.props.items && this.props.items.length && !this.scrollPositionSet) {

?


saveConfig(id, value) {
const data = JSON.parse(window.localStorage.getItem('ez-content-tree-state') || '{}');
const userId = window.eZ.helpers.user.getId();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it should be taken from window object. To the content tree userId is passed as a prop. It would be better to keep the consistency of data.

@lserwatka lserwatka merged commit 37b1323 into master Dec 23, 2020
@lserwatka lserwatka deleted the EZP-32248-retain-content-tree-state branch December 23, 2020 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants