Disable number grouping (thousands comma separators) by default in Vue node number widgets #5776
Conversation
dosubot
bot
added
the
size:L
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 09/26/2025, 12:13:34 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.test.ts
Outdated
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.test.ts
Outdated
Show resolved
Hide resolved
|
Test failures are expected due to the seed display in default workflow losing the thousands comma separators: https://59ad2721.comfyui-playwright-chromium.pages.dev/#?q=s:failed |
I don't think we need to update the litegraph interfaces, as litegraph won't be doing anything with the new widget options. The types will be defined in python -> node schemas -> Vue component props. |
… feedback Co-authored-by: DrJKL <[email protected]>
…esses review feedback Co-authored-by: DrJKL <[email protected]>
| it('displays numbers with commas when grouping enabled', () => { | ||
| const widget = createMockWidget(1000, 'int', { useGrouping: true }) | ||
| const wrapper = mountComponent(widget, 1000) | ||
|
|
||
| const input = getNumberInput(wrapper) | ||
| expect(input.value).toBe('1,000') | ||
| expect(input.value).toContain(',') | ||
| }) |
There was a problem hiding this comment.
idle thought: Very much not something I think is important to test right now, but does the PrimeVue component handle the internationalization of the separator?
There was a problem hiding this comment.
There's a locale prop https://primevue.org/inputnumber/#api.inputnumber.props.locale. I'm assuming they have intelligent default.
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.test.ts
Outdated
Show resolved
Hide resolved
…tNumberInput.test.ts Co-authored-by: Alexander Brown <[email protected]>
|
As mentioned here, test failure is expected now that comma is gone, so re-generating baseline. |
christian-byrne
added
the
New Browser Test Expectations
christian-byrne
deleted the
vue-nodes/widget/default-grouping-option
branch
…e node number widgets (#5776) ## Summary Makes the [useGrouping](https://primevue.org/inputnumber/#api.inputnumber.props.useGrouping) prop for number widgets disabled by default, aligning with the old UI (also requested via design). Node authors can still enable if they want by setting prop explicitly. ## Changes - **What**: Modified [WidgetInputNumberInput](https://primevue.org/inputnumber/) to disable `useGrouping` by default, requiring explicit opt-in via widget options - **Testing**: Added component tests covering value binding, component rendering, step calculations, and grouping behavior ## Review Focus UX impact on existing nodes that may have relied on default grouping behavior and test coverage for edge cases with precision calculations. ## Screenshots (if applicable) *Before*: <img width="1685" height="879" alt="Screenshot from 2025-09-25 11-34-34" src="https://github.com/user-attachments/assets/432097ab-203d-4f86-8ca0-721b27ee33de" /> *After*: <img width="1951" height="1175" alt="Screenshot from 2025-09-25 11-35-27" src="https://github.com/user-attachments/assets/74d35b62-612e-4dbf-b6e2-0ac17af03ea1" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5776-Disable-number-grouping-thousands-comma-separators-by-default-in-Vue-node-number-widget-2796d73d365081369ca6c155335d0d57) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <[email protected]> Co-authored-by: Alexander Brown <[email protected]> Co-authored-by: github-actions <[email protected]>
Summary
Makes the useGrouping prop for number widgets disabled by default, aligning with the old UI (also requested via design). Node authors can still enable if they want by setting prop explicitly.
Changes
useGroupingby default, requiring explicit opt-in via widget optionsReview Focus
UX impact on existing nodes that may have relied on default grouping behavior and test coverage for edge cases with precision calculations.
Screenshots (if applicable)
Before:
After:
┆Issue is synchronized with this Notion page by Unito