Skip to content

ISSUE-648: Change status and metadata classes to functional components#722

Merged
google-oss-prow[bot] merged 3 commits into
kubeflow:mainfrom
ada333:ISSUE-648
May 18, 2026
Merged

ISSUE-648: Change status and metadata classes to functional components#722
google-oss-prow[bot] merged 3 commits into
kubeflow:mainfrom
ada333:ISSUE-648

Conversation

@ada333

@ada333 ada333 commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

#648
This PR refactors these classes into functional components:

  1. CellMetadataEditor
  2. InlineCellsMetadata
  3. InlineMetadata
  4. StatusRunning
  5. StatusTerminated

For better code quality and readability there were some new hooks and separate component files added:
New hooks:

  1. useUpdateCellTags -- encapsulates all Kale cell metadata write operations
  2. useEditorPosition -- manages editor DOM positioning inside the active notebook cell
  3. useInlineCellsMetadata -- manages inline metadata portals, editor props, and active cell tracking
  4. useNotebookSignals -- connects/disconnects JupyterLab notebook signals (save, cell change, active cell, metadata)
  5. useLatestRef -- utility hook to avoid stale closures in signal handlers

New dialog components (extracted from CellMetadataEditor):

  1. BaseImageDialog
  2. CacheDialog
  3. GpuDialog (renamed from CellMetadataEditorDialog)

Other cleanup:

  1. Shared constants moved to constants.ts
  2. Dialog components organized into dialogs/ folder
  3. Removed dead commented-out code

Left Panel will be refactored in another PR.

also this PR fixes: #770

TESTING:

To test this I spinned up jupyter lab with these changes and jupyter lab from main and I compared the behaviour of UI. I recommend for testing of this PR to do the same thing.

@jesuino jesuino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello Adam,

Thanks for your work on this, I checked the other PR and it is looking good! Would it be possible to do all work in a single PR? It will even better for your to work on a single PR because when you get to dependent components you may have weird behavior related to state mismatch.

Thanks!

// limitations under the License.

import * as React from 'react';
// import { CSSProperties } from 'jss/css';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we take this opportunity to remove unused comments?

@google-oss-prow google-oss-prow Bot added size/XXL and removed size/M labels Apr 1, 2026
@ada333
ada333 marked this pull request as draft April 1, 2026 12:46
@ada333
ada333 force-pushed the ISSUE-648 branch 3 times, most recently from 670b8e4 to 055f7f1 Compare April 2, 2026 12:41
@ada333
ada333 marked this pull request as ready for review April 2, 2026 13:49
@ada333
ada333 requested a review from jesuino April 2, 2026 13:55
@ada333 ada333 changed the title ISSUE-648: Change status classes to functional components ISSUE-648: Change status and metadata classes to functional components Apr 20, 2026

@jesuino jesuino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello Adam,

Thanks again for your work on this. It is looking great and Kale is working as expected, but I noticed that the recently added output_path setting is not on your branch. Was it lost during some merge with main?

Thanks!

Split CellMetadataEditor into smaller functional components, extract
dialogs and hooks, convert status icons to functions, and fix page
scrolling when editing cell metadata.

Signed-off-by: Adam Maly <amaly@redhat.com>
…ges for some reason I cant remember

Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
@ada333

ada333 commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@jesuino hi, I squashed the commits and rebased this branch to have the latest changes. Also I added fix for #770 .

@jesuino jesuino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@google-oss-prow google-oss-prow Bot added the lgtm label May 18, 2026

@ederign ederign left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve
/lgtm

@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ederign

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow
google-oss-prow Bot merged commit ecce246 into kubeflow:main May 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[frontend] Labels are jumping up and down when editing cell metadata

3 participants