Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.8k
Fix: show 0 tabs instead of undefined in workspace tooltip (#18106)#18175
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
…b#18106)- Prefer reading layout from workspace.metadata (canonical saved state)- Use fallback pattern in fetch path (metadata then data)- Replace unsafe 'as any' with proper type guards- Add explicit Array.isArray() check for widgets- Fixes tooltip showing 'undefined' when layout data is missing
Thanks for making a pull request to jupyterlab! |
07c1ff3 to795c788Compare…tibility- Add explicit return type annotation to labelTitle()- Use tryGet() helper for safe property access- Handle both object and string JSON payloads- Support multiple property naming conventions- Improve fallback chain for metadata/data/id/name- Add non-null assertions where paths are validated- Better error handling for JSON parsing- More robust type definitions using Record<string, unknown>- Fixes issuejupyterlab#18106: tooltip showing undefined instead of tab count
795c788 to5a6f6c9Compare| ?.widgets?.length; | ||
| constworkspace=awaitapp.serviceManager.workspaces.fetch(workspaceId); | ||
| // Some workspace objects expose layout in metadata, others in data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why is that?
| // Prefer metadata, fall back to `.data`. | ||
| constrawLayout= | ||
| (workspaceasany)?.metadata?.['layout-restorer:data']?? | ||
| (workspaceasany)?.data?.['layout-restorer:data']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should not useany.any was the reason this bug crept in.
| // Helper: safely extract a field from rawMeta (supports string or object). | ||
| consttryGet=(obj:Record<string,unknown>|string,key:string)=>{ | ||
| if(typeofobj==='string'){ | ||
| returnundefined; | ||
| } | ||
| return(objasRecord<string,unknown>)[key]; | ||
| }; | ||
| // Candidate can be an object, or a JSON string containing layout info. | ||
| constcandidate= | ||
| tryGet(rawMeta,'layout-restorer:data')?? | ||
| tryGet(rawMeta,'layout-restorer')?? | ||
| tryGet(rawMeta,'layout')?? | ||
| rawMeta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should not be guessing.
| tryGet(rawMeta,'layout')?? | ||
| rawMeta; | ||
| typeLayoutShape={main?:{dock?:{widgets?:unknown[]}}}|undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a step in a good direction - we need to define the layout shape (but probably not in this package, but where it is actually being created) and import the type here.
| this._workspace.metadata.id, | ||
| (this._workspace.data['layout-restorer:data']asany)?.main?.dock | ||
| ?.widgets?.length, | ||
| this._workspace.metadata['last_modified'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you have an example of when last modified was wrong?
Fix: Tooltip in Workspaces Manager Shows Undefined Rather Than Number of Tabs
Closes#18106
Problem:
The workspace tooltip could display "undefined" because layout information was read without checks and passed directly into the translation function.
Solution:
Files modified:
Manual verification:
Tooltip now shows “N tabs” instead of “undefined”.
Screenshot: (
#18106.pdf
)