Open
Conversation
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Member
|
@ada333 keeps those PR's coming, I'm just waiting to branch for 2.0 to start to review and merge those. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors LeftPanel into functional component using custom hooks. This make the component much more simple - from 800 lines to 300. The 6 hooks introduced in here ->
useNotebookMetadatauseNotebookLoadercurrentChangedsignal and runs the async loading sequence (wait for session, fetch base image/namespace/experiments via RPC, read.ipynbmetadata, merge defaults).useNotebookMetadataPersistence.ipynbfile whenever the state changes.useEnableByDefaultEffectuseKfpStatuskfp.pingevery 30s and returns the connection status for the header badge.useDeploymentOther important changes:
Types extracted to LeftPanelTypes.ts
DeployType, IExperiment, IKaleNotebookMetadata, DefaultState, and NEW_EXPERIMENT moved to a dedicated types file. LeftPanel.tsx re-exports them so existing imports from other files continue to work without changes.
Toolbar decoupled from the class instance
kaleToolbar.ts no longer imports KubeflowKaleLeftPanel directly. Instead it defines an ILeftPanelHandle interface (triggerCompile, triggerRun, isKaleEnabled) and exposes setLeftPanelCallbacks. The function component registers its callbacks via a useEffect, eliminating the React ref that was previously threaded through widget.tsx.
Utility functions made standalone
getNotebookFileName, sanitizePipelineName, and getNotebookPath were extracted from class methods into plain functions in useNotebookLoader.ts — they had no dependency on this and are now easier to unit-test.
I tested this by simply running JupyterLab and trying Left Panel in different ways. But this definitely needs thorough testing - there are lot of different scenarios.