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

Check for hidden items before re-rendering #531

Merged
merged 9 commits into from
Feb 9, 2023

Conversation

alec-kr
Copy link
Contributor

@alec-kr alec-kr commented Feb 5, 2023

This PR fixes issue #13677 from jupyterlab.

Issue

Hidden items are still being re-rendered, despite the fact that they shouldn't be.

Solution

Added a check to onUpdateRequest to determine if an item is hidden, before re-rendering it. This will prevent hidden items from being re-rendered, thus solving the issue.

@welcome
Copy link

welcome bot commented Feb 5, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@krassowski krassowski added enhancement New feature or request performance Addresses performance labels Feb 6, 2023
@alec-kr alec-kr requested a review from krassowski February 7, 2023 00:24
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @alec-kr

Could you trigger an update when the command palette is visible? This will ensure we apply the command palette updates shunted when it was hidden.

protected onAfterShow(msg: Message): {
  this.update();
  super.onAfterShow(msg);
}

Reference inherited method:

protected onAfterShow(msg: Message): void {}

@alec-kr
Copy link
Contributor Author

alec-kr commented Feb 7, 2023

Sure! I've updated the method. Is this correct?

Edit: I definitely didn't do this right. Where exactly should this change be made? Sorry about the confusion

@fcollonval
Copy link
Member

Sure! I've updated the method. Is this correct?

Edit: I definitely didn't do this right. Where exactly should this change be made? Sorry about the confusion

Sorry my fault I mislead you by referring to the inherited method. You need to add my snippet within the CommandPalette class; for example there:

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @alec-kr

@alec-kr
Copy link
Contributor Author

alec-kr commented Feb 8, 2023

Thank you for your patience 😄

@fcollonval fcollonval merged commit f134216 into jupyterlab:main Feb 9, 2023
@welcome
Copy link

welcome bot commented Feb 9, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request performance Addresses performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants