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

TagBox: Fix tag label display when valueExpr is set to function and hideSelectedItems is enabled (T1234032) (Draft) #27521

Closed
wants to merge 31 commits into from

Conversation

jdvictoria
Copy link
Contributor

No description provided.

@jdvictoria jdvictoria added the 24_1 label Jun 5, 2024
@jdvictoria jdvictoria self-assigned this Jun 5, 2024
@jdvictoria jdvictoria requested a review from a team June 6, 2024 10:52
@ksercs ksercs requested a review from AlexanderMoiseev June 10, 2024 08:05
@AlexanderMoiseev
Copy link
Contributor

AlexanderMoiseev commented Jun 12, 2024

JFYI: there is a way to fix it on selection.strategy level i believe, but, as long this bevavior with hidden items is possible only with tagBox, i thing this WA will do. I was looking for solution with selecteion first, but decided not to tamper with base functionality

Copy link
Contributor

@ksercs ksercs left a comment

Choose a reason for hiding this comment

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

[missed scenarios, local solution]

I agree with Alex - this issue is mostly expected to be fixed not on a TagBox level. I see that proposed solution works in the target scenario but I cannot find a reason "why ideologically it should work this way?".

I believe we cannot accept this solution because it assumes that whole hideSelectedItems prop processing was incorrect, but I see it worked well when valueExpr is not a function => the reason of the issue is related to the valueExpr option processing and mostly it's not on a TagBox level.

Previously algorithm checked if necessary items are already loaded (are in plain items) and then just render it or additionally loaded dependent on an answer. After your change the code always believes items are already loaded and just renders them - sound not correct. Example of a broken scenario: just set some value in initial config.

Let's try to find a real reason this way:

  • create two sample pages - with broken behavior when valueExpr is a function and with a correct behavior when valueExpr is a string
  • debug both and find the difference - when exactly we get some unexpected result (mostly it's related to list selectedItems or like this)
  • find why we get this incorrect result and fix (mostly somewhere in collection strategy)

Please don't forget to write separate tests additionally, at least for the scenario mentioned above.
Also wanna remind you that we will need to write a test for place where the fix is applied (for collection if the fix is on collection level).

const tagBox = $tagBox.dxTagBox('instance');
const $list = tagBox._list.$element();

if(changeAtRuntime) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[excess operation]

This condition is excess: if initially property is true, setting it to true at runtime will do nothing

tagBox.option('hideSelectedItems', true);
}

$($list.find('.dx-list-item').eq(0)).trigger('dxclick');
Copy link
Contributor

Choose a reason for hiding this comment

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

[repeatable code style mistake]

Constants please!

… span and input in Vue template (DevExpress#27620)

Signed-off-by: Nikki Gonzales <[email protected]>
Co-authored-by: Rochmar Nicolas (DevExpress) <[email protected]>
…Esc key is pressed (T1219785) (DevExpress#27633)

Co-authored-by: Alyar <>
…scrolling and grouping are used (T1221369) (DevExpress#27642)

Co-authored-by: williamvinogradov <[email protected]>
@jdvictoria jdvictoria requested review from a team as code owners June 22, 2024 10:55
@jdvictoria jdvictoria closed this Jun 22, 2024
@jdvictoria jdvictoria changed the title TagBox: Fix tag label display when valueExpr is set to function and hideSelectedItems is enabled (T1234032) TagBox: Fix tag label display when valueExpr is set to function and hideSelectedItems is enabled (T1234032) (Draft) Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.