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 TagList formwidget #1267

Merged
merged 9 commits into from
Dec 19, 2024
Merged

Fix TagList formwidget #1267

merged 9 commits into from
Dec 19, 2024

Conversation

mjauvin
Copy link
Member

@mjauvin mjauvin commented Dec 5, 2024

The TagList formwidget was not working properly in relation mode if the relation was not using a pivot table (e.g. morphMany/hasMany)

All modes did not update the model field or relation when all tags had been cleared.

Fixes #1223

@mjauvin mjauvin added maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer labels Dec 5, 2024
@mjauvin mjauvin added this to the 1.2.8 milestone Dec 5, 2024
@mjauvin mjauvin requested a review from LukeTowers December 5, 2024 18:50
@mjauvin mjauvin self-assigned this Dec 5, 2024
@mjauvin mjauvin requested a review from bennothommo December 7, 2024 13:12
@mjauvin
Copy link
Member Author

mjauvin commented Dec 7, 2024

@bennothommo I just saw you started a PR on TagList as well, thought you might want to review this one.

@mjauvin mjauvin marked this pull request as draft December 7, 2024 13:53
@bennothommo
Copy link
Member

@mjauvin it's probably worth adding an example in the Test plugin if there isn't one already, if adding the unit tests becomes too difficult.

@mjauvin
Copy link
Member Author

mjauvin commented Dec 7, 2024

yeah, that's what I'm going to do for now.

@mjauvin mjauvin marked this pull request as ready for review December 7, 2024 15:00
@mjauvin
Copy link
Member Author

mjauvin commented Dec 8, 2024

tested working with the following modes using Winter.Test plugin in Post & Record models:

  • relation (belongsToMany, hasMany, morphMany)
  • array (useKey = true/false)
  • string (useKey = true/false)

All the above tests also include clearing all values and making sure the model field or relation was updated accordingly.

@mjauvin mjauvin changed the title Fix broken taglist formwidget in relation mode Fix TagList formwidget Dec 10, 2024
@mjauvin
Copy link
Member Author

mjauvin commented Dec 18, 2024

@LukeTowers Anything more before merging this?

@LukeTowers
Copy link
Member

@mjauvin @bennothommo Has it been fully tested?

@mjauvin
Copy link
Member Author

mjauvin commented Dec 18, 2024

@mjauvin @bennothommo Has it been fully tested?

Yes, see previous comment above.

@AIC-BV
Copy link
Contributor

AIC-BV commented Dec 19, 2024

Can confirm this fixes the problem
Thanks!

@LukeTowers LukeTowers removed the needs review Issues/PRs that require a review from a maintainer label Dec 19, 2024
@LukeTowers LukeTowers merged commit 20a73ff into develop Dec 19, 2024
13 checks passed
@LukeTowers LukeTowers deleted the taglist-relation branch December 19, 2024 11:31
@AIC-BV
Copy link
Contributor

AIC-BV commented Dec 19, 2024

image
Oh it seems to generate 2 tags instead of one when creating a tag for the first time

When removing all tags, they also save as an empty string
image

When adding tags
image

@mjauvin
Copy link
Member Author

mjauvin commented Dec 19, 2024

@AIC-BV can you show your tag field definition and relation?

@AIC-BV
Copy link
Contributor

AIC-BV commented Dec 19, 2024

@mjauvin
There is no relation

tags:
    label: Tags
    type: taglist
    customTags: false
    options:
        nieuw: Nieuw
        best: Beste deal
    span: auto

@LukeTowers
Copy link
Member

@AIC-BV can you open an issue for that so that we don't release 1.2.8 until it's fixed?

@AIC-BV
Copy link
Contributor

AIC-BV commented Dec 20, 2024

#1275

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PRs that fix bugs, are translation changes or make only minor changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TagList not saving an empty value
4 participants