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(module: Select) bind value change lock, tags and labels fix, overlay reposition #1191

Merged

Conversation

anddrzejb
Copy link
Member

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • Bundle size optimization
  • Performance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

Fixes #1171

💡 Background and solution

Commit 25c3dcb

During my tests, it came out that if I had a class that the label was attached to a property that it had only a getter, then Select was crashing. The source of problems was the PR #1168. The _setLabel was prepared regardless of the type of select. The property that is attached to label needs to have a setter only for SelectMode.Tags.

Commit d4e2966

Several fixes to solve #1171. The result it this:

select_bindValueLockFix

7c7d481

Update of the overlay position was not really effective, as it was doing its updates before all the relevant changes were pushed to DOM. Adding Task.Delay(1) solved this issue. I am not really happy with it. The other option would be maybe to use ResizeObserver but there might be a problem with browser compatibility.

Other condierations:

All operations done in mehtods EvaluateValueChangedOutsideComponent and EvaluateValuesChangedOutsideComponent are basically needed only when Value and Values are changed outside of the component. I imagine this is going to be a much less often scenario in comparison to when they are changed by the component. At this stage, when values are changed by the component, the aforementioned methods are not necessary. This is mostly a performance issue when dealing with multiple/tags mode. But it probably would be beneficial to have some kind of a way to generate a hash or some other way to be able to compare if values were changed internally or externally. If external change is detected, then run the evaluation methods.

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@codecov-io
Copy link

Codecov Report

Merging #1191 (7c7d481) into master (fd388c5) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1191      +/-   ##
=========================================
- Coverage    6.38%   6.36%   -0.02%     
=========================================
  Files         439     439              
  Lines       25074   25141      +67     
=========================================
  Hits         1600    1600              
- Misses      23474   23541      +67     
Impacted Files Coverage Δ
components/select/Select.razor.cs 0.00% <0.00%> (ø)
components/select/internal/SelectContent.razor 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd388c5...7c7d481. Read the comment docs.

Copy link
Member

@ElderJames ElderJames left a comment

Choose a reason for hiding this comment

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

LGTM.

@ElderJames ElderJames merged commit 844c6eb into ant-design-blazor:master Mar 2, 2021
@anddrzejb anddrzejb deleted the selectBindValueChangeLock branch March 3, 2021 06:57
zxyao145 pushed a commit to zxyao145/ant-design-blazor that referenced this pull request Mar 20, 2021
…rlay reposition (ant-design-blazor#1191)

* fix(module:select) _setLabel not needed for non-tags

fix for exception when label is a property with getter only

* fix(module:select): bind-Value lock if changed outside component

* fix(module:Select): overlay reposition SelectContent resize
ElderJames pushed a commit that referenced this pull request Apr 23, 2022
…rlay reposition (#1191)

* fix(module:select) _setLabel not needed for non-tags

fix for exception when label is a property with getter only

* fix(module:select): bind-Value lock if changed outside component

* fix(module:Select): overlay reposition SelectContent resize
ElderJames pushed a commit that referenced this pull request Apr 30, 2022
…rlay reposition (#1191)

* fix(module:select) _setLabel not needed for non-tags

fix for exception when label is a property with getter only

* fix(module:select): bind-Value lock if changed outside component

* fix(module:Select): overlay reposition SelectContent resize
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.

[Select] Can't change the binding value after changing the @bind-Value
3 participants