Skip to content

KDS-547: Tokenization of Python scripting – Temporary Values Panel #39

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.

Sign up for GitHub

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

Jump to bottom
Open
senemdilli wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KDS-547: Tokenization of Python scripting – Temporary Values Panel #39

senemdilli wants to merge 1 commit into from

Conversation

@senemdilli
Copy link

@senemdilli senemdilli commented Jan 8, 2026

No description provided.

Copilot AI review requested due to automatic review settings January 8, 2026 08:15
@senemdilli senemdilli requested a review from a team as a code owner January 8, 2026 08:15
Copilot AI reviewed Jan 8, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements tokenization updates for the Python scripting Temporary Values Panel, migrating from legacy KNIME component styles to the KNIME Design System (KDS). The changes focus on updating CSS styling to use KDS design tokens and replacing the old Button component with KdsButton.

  • Migrated CSS from hardcoded values to KDS design tokens for consistent theming
  • Replaced @knime/components Button with @knime/kds-components KdsButton
  • Updated table header/body styling to follow KDS spacing and visual patterns

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

File Description
PythonWorkspaceHeader.vue Updated table header styling with KDS tokens, changed sticky positioning structure, added KDS font and color tokens
PythonWorkspaceBody.vue Applied KDS spacing, typography, and hover state styling to table rows
PythonWorkspace.vue Migrated Button to KdsButton, replaced hardcoded spacing/colors with KDS tokens throughout
package.json Added @knime/kds-components dependency and downgraded @vueuse/core

org.knime.python3.scripting.nodes/js-src/src/components/PythonWorkspace.vue
:disabled="!resetButtonEnabled"
@click="resetWorkspace"
>
Reset values
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The button text 'Reset values' is duplicated between the 'label' prop (line 80) and the slot content. Remove the slot content since the label prop already provides the button text.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@ChristianAlbrecht ChristianAlbrecht Jan 20, 2026

Choose a reason for hiding this comment

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

No slots in KdsButtons anymore!

@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from cf613ca to a7bea4a Compare January 8, 2026 08:21
Copilot AI review requested due to automatic review settings January 16, 2026 11:51
@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from a7bea4a to ef340ba Compare January 16, 2026 11:51
Copilot AI reviewed Jan 16, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch 2 times, most recently from c2d35a6 to 8a70956 Compare January 16, 2026 15:35
Copilot AI review requested due to automatic review settings January 16, 2026 15:35
Copilot AI reviewed Jan 16, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from 8a70956 to 8e86d30 Compare January 20, 2026 08:17
Copilot AI review requested due to automatic review settings January 20, 2026 08:49
@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from 8e86d30 to 08ccdb4 Compare January 20, 2026 08:49
Copilot AI reviewed Jan 20, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

HedgehogCode
HedgehogCode requested changes Jan 20, 2026
View reviewed changes
@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from 08ccdb4 to d347887 Compare January 20, 2026 14:30
Copilot AI review requested due to automatic review settings January 20, 2026 14:38
@HedgehogCode HedgehogCode force-pushed the KDS-547-python-scripting-temp-values branch 2 times, most recently from d347887 to 3621f4b Compare January 20, 2026 14:39
Copilot AI reviewed Jan 20, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

org.knime.python3.scripting.nodes/js-src/src/components/PythonWorkspace.vue Outdated
const rootResizeCallback = useDebounceFn((entries) => {
const rect = entries[0].contentRect;
totalWidth.value = rect.width;
totalWidth.value = rect.width - paddingAroundTable * 2;
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The constant paddingAroundTable doesn't accurately describe what it represents. Consider renaming to horizontalPadding or sidePadding since it's multiplied by 2 to account for both left and right padding.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings January 21, 2026 14:02
@senemdilli senemdilli force-pushed the KDS-547-python-scripting-temp-values branch from 3621f4b to 80561f3 Compare January 21, 2026 14:02
Copilot AI reviewed Jan 21, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

org.knime.python3.scripting.nodes/js-src/src/components/PythonWorkspace.vue
const resizeContainer = ref<HTMLElement | null>(null);
const totalWidth: Ref<number> = ref(0);
const headerWidths = ref<ColumnSizes>([100, 100, 100]);
// TODO KDS-690: Replace it once the JS tokens are available
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The hardcoded magic number 8 should be documented to explain how it corresponds to the CSS padding value. The comment mentions JS tokens will replace this, but it's unclear how this value relates to var(--kds-spacing-container-0-5x) used in the CSS.

Suggested change
// TODO KDS-690: Replace it once the JS tokens are available
// TODO KDS-690: Replace this hardcoded value once JS spacing tokens are available.
// 8px corresponds to `var(--kds-spacing-container-0-5x)` used as the horizontal padding
// in the `.workspace` CSS rule below. Keep this value in sync with that CSS token.

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 21, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
60.0% Coverage on New Code (required ≥ 85%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristianAlbrecht ChristianAlbrecht ChristianAlbrecht left review comments

Copilot code review Copilot Copilot left review comments

@HedgehogCode HedgehogCode HedgehogCode requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@senemdilli @HedgehogCode @ChristianAlbrecht