Skip to content

Conditions not working (partially)#14488

Merged
rijkvanzanten merged 12 commits intomainfrom
fix/v-form-conditions
Jul 21, 2022
Merged

Conditions not working (partially)#14488
rijkvanzanten merged 12 commits intomainfrom
fix/v-form-conditions

Conversation

@br41nslug
Copy link
Copy Markdown
Member

@br41nslug br41nslug commented Jul 18, 2022

Description

Fixes #14285
Fixes #14392

Not all properties changed by conditions were not propagated fully after performance optimizations #14188 .
I've reverted the formFields/fieldMeta split in favor of a fieldMap object for better separated update tracking than in an array.
To keep the v-form signature the same as it was i did not update the way it passes it's properties down to nested forms which adds a slight overhead for re-formatting the data on each recursion.

Note: There is a little regression on performance noticeable on huge collections (>100 inputs compounded by accordions if used)

Testing

Database: data.zip (default login)

  1. Various conditions Conditional field stops working on CSV field #14392 Conditions not working (partially) #14285 [Works]
  2. Dashboard time series conditional Conditions not working (partially) #14285 [Works]
    image
  3. huge collection (check performance) [Feels like some regression here]
  4. absurd nesting [Fixed]
    image
  5. grid style [Fixed]

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated

@br41nslug br41nslug marked this pull request as ready for review July 20, 2022 10:06
@br41nslug br41nslug requested review from azrikahar and licitdev July 20, 2022 10:06
Copy link
Copy Markdown
Contributor

@azrikahar azrikahar left a comment

Choose a reason for hiding this comment

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

Conditions works nicely from what I've tested! (nested groups, conditions based on field in another group, condition based on field in parent group, conditions for field options, time series count/countDistinct primarykey selection etc)

One minor issue with user form, which I believe is related to the change in /modules/users/routes/item.vue. Though that does make me wonder is fieldsFiltered doing anything before your change? 🤔


To reproduce the user form issue

  1. Create 2 new string fields in directus_user, named test and test2.
  2. Then re-order test2 to above test. This step is important because it was the main trigger of such issue reported in #8560 (comment).

Result:

chrome_MXrcy5June

How it looks like before this PR

chrome_VYDYYceQ3j.mp4

How it looks like after this PR

chrome_531yHjoWwr.mp4

@br41nslug
Copy link
Copy Markdown
Member Author

Nice catch Azri! 🙌 You're right and as the changes to /modules/users/routes/item.vue are not technically needed or in the scope of this PR i have reverted that

@rijkvanzanten rijkvanzanten self-assigned this Jul 21, 2022
@rijkvanzanten rijkvanzanten merged commit 8bc098c into main Jul 21, 2022
@rijkvanzanten rijkvanzanten deleted the fix/v-form-conditions branch July 21, 2022 18:39
@rijkvanzanten rijkvanzanten added this to the Next Release milestone Jul 21, 2022
qborisb pushed a commit to qdentity/directus that referenced this pull request Jul 22, 2022
* changed way of storing the formfield data in v-form

* hacky translation implementation

* updated translations without bombing the performance

* removed debug code and refactored initial implementation

* removed redundant useFormFields in users route

* making proper use of useFormField results to not break the grid layout

* removed limitation for conditions

* Fix selection of foreign keys for value field in time series panel

* Revert "removed redundant useFormFields in users route"

This reverts commit c6f4f23.

Co-authored-by: ian <licitdev@gmail.com>
Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com>
Co-authored-by: rijkvanzanten <rijkvanzanten@me.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conditional field stops working on CSV field Conditions not working (partially)

4 participants