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

Update title in AccordionPanel #249

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Conversation

hbcarlos
Copy link
Member

Solves #248

I'm not sure if this is the right way of updating the title.

I'm not clearing the signals in the dispose method because it's done in the parent class Widget.

Code changes

This PR adds a new handler to listen for changes in the titles of the accordion panel's children and replaces the corresponding title's HTMLElement.

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 @hbcarlos

The logic is not the same than the one in the tab bar or the menu bar. But in this case it sounds better as the title are not rendered in a separated container as the previous case.

I added a comment in the code.

packages/widgets/src/accordionlayout.ts Outdated Show resolved Hide resolved
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.

LGTM thanks @hbcarlos

@blink1073 blink1073 added the bug Something isn't working label Oct 14, 2021
@blink1073
Copy link
Contributor

@hbcarlos, can you please add tests to the existing accordianlayout and accordianpanel suites?

@hbcarlos
Copy link
Member Author

Sure! I'm on it!

@hbcarlos
Copy link
Member Author

@blink1073 What test should I add to the layout?

@blink1073
Copy link
Contributor

The panel should be enough, it is exercising the layout under the hood. Thank you!

@blink1073 blink1073 merged commit 82e45aa into jupyterlab:master Oct 14, 2021
@hbcarlos hbcarlos deleted the update_title branch October 14, 2021 21:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants