Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: inputs with extends TuiControl has CD problems for writeValue of empty value #10220

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nsbarsukov
Copy link
Member

Fixes #10202

@nsbarsukov nsbarsukov self-assigned this Jan 23, 2025
Copy link

lumberjack-bot bot commented Jan 23, 2025

Failed tests ❌

Before (main) ← Diff → After (local)

tests-core-data-list-data-list.pw-DataList-Custom-list-chromium-retry2/01-data-list.diff.png
tests-core-data-list-data-list.pw-DataList-Custom-list-webkit-retry2/01-data-list.diff.png
tests-core-textfield-textf-460ef-vity-on-hover-disable-state-chromium-retry2/textfield-disabled-hover.diff.png
tests-core-textfield-textf-6d0fb-vity-on-hover-invalid-state-chromium-retry2/textfield-invalid-hover.diff.png
tests-core-textfield-textf-df024-ity-on-hover-readonly-state-chromium-retry2/textfield-readonly-hover.diff.png
tests-demo-demo.pw-Demo-components-textfield-chromium-retry2/components-textfield/2.diff.png
tests-demo-demo.pw-Demo-components-textfield-webkit-retry2/components-textfield/2.diff.png

(updated for commit 194d1e0)

Comment on lines +62 to +65
cy.get('tui-textfield').compareSnapshot({
name: 'input-number-write-value-empty-string',
cypressScreenshotOptions: {padding: 8},
});
Copy link
Member Author

Choose a reason for hiding this comment

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

Before fix

write-value-empty-string cy ts-input-number-write-value-empty-string

Copy link
Member Author

Choose a reason for hiding this comment

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

After fix

write-value-empty-string cy ts-input-number-write-value-empty-string

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 65.76%. Comparing base (0d58a2f) to head (194d1e0).

Files with missing lines Patch % Lines
.../components/input-number/input-number.component.ts 0.00% 5 Missing ⚠️
...s/core/components/textfield/textfield.directive.ts 25.00% 3 Missing ⚠️
...ternational/input-phone-international.component.ts 78.57% 0 Missing and 3 partials ⚠️
projects/cdk/classes/control.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10220      +/-   ##
==========================================
- Coverage   65.78%   65.76%   -0.02%     
==========================================
  Files        1240     1240              
  Lines       16223    16228       +5     
  Branches     2309     2310       +1     
==========================================
+ Hits        10672    10673       +1     
+ Misses       5404     5362      -42     
- Partials      147      193      +46     
Flag Coverage Δ
summary 65.76% <50.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jan 23, 2025

Visit the preview URL for this PR (updated for commit 194d1e0):

https://taiga-previews-demo--pr10220-control-cd-bug-demo-lqztdzzi.web.app

(expires Tue, 28 Jan 2025 06:41:06 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 73dddc3c665194f3e11f18c16aeb71af4c289c37

Copy link

bundlemon bot commented Jan 23, 2025

BundleMon

Files updated (1)
Status Path Size Limits
demo/browser/main.(hash).js
309.8KB (+47B +0.01%) +10%
Unchanged files (4)
Status Path Size Limits
demo/browser/vendor.(hash).js
260.28KB +10%
demo/browser/runtime.(hash).js
46.5KB +10%
demo/browser/styles.(hash).css
20.93KB +10%
demo/browser/polyfills.(hash).js
11.18KB +10%

Total files change +49B +0.01%

Groups updated (1)
Status Path Size Limits
demo/browser/*..js
7.83MB (+79B 0%) -

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Copy link
Contributor

github-actions bot commented Jan 23, 2025

Playwright test results

failed  7 failed
passed  2066 passed
skipped  83 skipped

Details

report  Open report ↗︎
stats  2156 tests across 73 suites
duration  18 minutes, 29 seconds
commit  194d1e0

Failed tests

chromium › tests/core/data-list/data-list.pw.spec.ts › DataList › Custom list
webkit › tests/core/data-list/data-list.pw.spec.ts › DataList › Custom list
chromium › tests/core/textfield/textfield.pw.spec.ts › Textfield › interactivity on hover › disable state
chromium › tests/core/textfield/textfield.pw.spec.ts › Textfield › interactivity on hover › readonly state
chromium › tests/core/textfield/textfield.pw.spec.ts › Textfield › interactivity on hover › invalid state
chromium › tests/demo/demo.pw.spec.ts › Demo › /components/textfield
webkit › tests/demo/demo.pw.spec.ts › Demo › /components/textfield

Skipped tests

webkit › tests/addon-commerce/input-card-group.pw.spec.ts › InputCardGroup › Examples › input card grouped with validation
webkit › tests/addon-commerce/input-card-group.pw.spec.ts › InputCardGroup › Examples › input card grouped with saved cards
webkit › tests/addon-commerce/input-card-group.pw.spec.ts › InputCardGroup › Examples › expired field should be clickable after reset of prefilled value
webkit › tests/addon-doc/navigation.pw.spec.ts › Navigation › anchor links navigation works › scroll to "tui-doc-example"
webkit › tests/addon-doc/navigation.pw.spec.ts › Navigation › anchor links navigation works › scroll to "tui-doc-code"
webkit › tests/addon-doc/navigation.pw.spec.ts › Navigation › anchor links navigation works › scroll after click on link with anchor
webkit › tests/addon-doc/navigation.pw.spec.ts › Navigation › getting started / [light mode]
webkit › tests/addon-doc/navigation.pw.spec.ts › Navigation › getting started / [dark mode]
webkit › tests/core/data-list/data-list.pw.spec.ts › DataList › Submenu
webkit › tests/core/data-list/data-list.pw.spec.ts › DataList › Form control
webkit › tests/core/data-list/data-list.pw.spec.ts › DataList › Complex
webkit › tests/core/data-list/data-list.pw.spec.ts › DataList › Options with long text
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Dialog with confirmation works › Prompt
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Dialog with confirmation works › Pristine form does not show confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Dialog with confirmation works › Dirty form shows confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Dialog with confirmation works › Form is reset to pristine
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › A dialog and a nested dialog are open correctly
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Mobile dialog works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Dialog with directive works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Show simple
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 320x480 › Show simple + custom button
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Dialog with confirmation works › Prompt
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Dialog with confirmation works › Pristine form does not show confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Dialog with confirmation works › Dirty form shows confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Dialog with confirmation works › Form is reset to pristine
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › A dialog and a nested dialog are open correctly
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Mobile dialog works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Dialog with directive works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Show simple
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 720x900 › Show simple + custom button
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Dialog with confirmation works › Prompt
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Dialog with confirmation works › Pristine form does not show confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Dialog with confirmation works › Dirty form shows confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Dialog with confirmation works › Form is reset to pristine
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › A dialog and a nested dialog are open correctly
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Mobile dialog works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Dialog with directive works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Show simple
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1024x900 › Show simple + custom button
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Dialog with confirmation works › Prompt
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Dialog with confirmation works › Pristine form does not show confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Dialog with confirmation works › Dirty form shows confirmation
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Dialog with confirmation works › Form is reset to pristine
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › A dialog and a nested dialog are open correctly
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Mobile dialog works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Dialog with directive works
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Show simple
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › 1620x1024 › Show simple + custom button
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › Prompt - 320x480
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › Prompt - 720x900
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › Prompt - 1024x900
webkit › tests/core/dialogs/dialogs.pw.spec.ts › Dialogs › Prompt - 1620x1024
webkit › tests/core/surface/surface.pw.spec.ts › Surface › Layers
webkit › tests/kit/carousel/carousel.pw.spec.ts › Carousel › should show next item after drag
webkit › tests/kit/dropdown-hover/dropdown-hover.pw.spec.ts › DropdownHover › Examples › With DropdownMobile › Closes dropdown on click on overlay
webkit › tests/kit/dropdown-hover/dropdown-hover.pw.spec.ts › DropdownHover › Examples › With DropdownMobile › Opens mobile version of dropdown on the 2nd time click
webkit › tests/kit/input-number/input-number.pw.spec.ts › InputNumber › API › [prefix] & [postfix] props › [prefix]="$" & [postfix]="" › textfield does not contain any digit (only suffixes) => clear inputNumber.textfield value on blur
webkit › tests/kit/input-number/input-number.pw.spec.ts › InputNumber › API › [prefix] & [postfix] props › [prefix]="" & [postfix]="kg" › textfield does not contain any digit (only suffixes) => clear inputNumber.textfield value on blur
webkit › tests/kit/input-number/input-number.pw.spec.ts › InputNumber › API › [prefix] & [postfix] props › [prefix]="$" & [postfix]="kg" › textfield does not contain any digit (only suffixes) => clear inputNumber.textfield value on blur
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › change selected range on click › click on the beginning of the track changes only nearest (left) slider
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › change selected range on click › click on the end of the track changes only nearest (right) slider
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › change selected range on click › click between two thumbs triggers only nearest thumb
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › basic range (from 0 to 1000 with 250 steps). Initial value [0, 250] › pressing of Arrow Right increases by one step (after focus on right slider)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › basic range (from 0 to 1000 with 250 steps). Initial value [0, 250] › pressing of Arrow Right increases by one step (after focus on left slider)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › basic range (from 0 to 1000 with 250 steps). Initial value [0, 250] › pressing of Arrow Left decreases by one step (after setting right thumb active via click)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › basic range (from 0 to 1000 with 250 steps). Initial value [0, 250] › cannot set left thumb more than right thumb (by Arrow Right)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › basic range (from 0 to 1000 with 250 steps). Initial value [0, 250] › cannot set right thumb less than left thumb (by ArrowLeft)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › range with keySteps (from 0 to 1M) with 8 steps. Initial value [0, 100_000] › ArrowUp increases value of the focused slider
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › range with keySteps (from 0 to 1M) with 8 steps. Initial value [0, 100_000] › ArrowDown decreases value of the focused slider
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › range with keySteps (from 0 to 1M) with 8 steps. Initial value [0, 100_000] › cannot set position of the LEFT slider MORE THAN position of the RIGHT slider (by ArrowUp)
webkit › tests/kit/range/range.pw.spec.ts › TuiRange › examples page › keyboard interactions › range with keySteps (from 0 to 1M) with 8 steps. Initial value [0, 100_000] › cannot set position of the RIGHT slider LESS THAN position of the LEFT slider (by ArrowDown)
webkit › tests/kit/slider/slider.pw.spec.ts › Slider › programmatically change value › ngModel › decrease value by 1 step
webkit › tests/kit/slider/slider.pw.spec.ts › Slider › programmatically change value › ngModel › increase value by 1 step
webkit › tests/kit/slider/slider.pw.spec.ts › Slider › programmatically change value › ngModel › increase value by 2 steps
webkit › tests/kit/tabs/tabs.pw.spec.ts › Tabs › Examples › complex › no extra margin after the last tab
webkit › tests/kit/tabs/tabs.pw.spec.ts › Tabs › Examples › complex › shows only a single dropdown for the nested item (with [tuiDropdown]) inside more section
webkit › tests/layout/navigation.pw.spec.ts › Navigation › Show settings in aside
webkit › tests/layout/navigation.pw.spec.ts › Navigation › Show hint in aside
webkit › tests/layout/navigation.pw.spec.ts › Navigation › A12y
webkit › tests/legacy/input-time/input-time.pw.spec.ts › InputTime › API › items are passed › the dropdown is visible when the input is focused
webkit › tests/legacy/input-time/input-time.pw.spec.ts › InputTime › API › items are passed › the dropdown is configured for s size
webkit › tests/legacy/input-time/input-time.pw.spec.ts › InputTime › API › items are passed › the dropdown is configured for m size
webkit › tests/legacy/input-time/input-time.pw.spec.ts › InputTime › API › items are passed › the dropdown is configured for l size

@nsbarsukov nsbarsukov marked this pull request as ready for review January 23, 2025 10:25
@nsbarsukov nsbarsukov requested a review from a team as a code owner January 23, 2025 10:25
@nsbarsukov nsbarsukov requested review from MarsiBarsi, waterplea, vladimirpotekhin and mdlufy and removed request for a team January 23, 2025 10:25
protected valueChangeEffect = effect(() => {
this.onChange(this.unmask(this.masked()));
// Host binding `host: {'[value]': 'masked()'}` has change detection problem with empty string
this.render.setProperty(this.el, 'value', this.masked());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we only need it for value property we can make it a host directive to hide this hideous logic inside one place, if we cannot figure out why this issue is happening.

@@ -24,6 +25,11 @@ export class TuiTextfieldBase<T> implements OnChanges {
private readonly focused = signal<boolean | null>(null);

protected readonly control = inject(NgControl, {optional: true});
protected readonly tuiControl = inject(TuiControl, {optional: true});
protected readonly empty = computed(
() => (this.tuiControl?.rawValue() ?? this.value()) === '',
Copy link
Member Author

@nsbarsukov nsbarsukov Jan 27, 2025

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Angular 16 bug ?

Снимок экрана 2025-01-27 в 11 07 21 Снимок экрана 2025-01-27 в 11 06 40

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, it is not reproducible for angular@17

@nsbarsukov nsbarsukov changed the title fix: inputs with extends TuiControl has CD problems for writeValue of empty value [WIP] fix: inputs with extends TuiControl has CD problems for writeValue of empty value Jan 27, 2025
@nsbarsukov nsbarsukov changed the title [WIP] fix: inputs with extends TuiControl has CD problems for writeValue of empty value fix: inputs with extends TuiControl has CD problems for writeValue of empty value Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

🐞 - Inputs with extends TuiControl has CD problems for writeValue of "empty" value
3 participants