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

[notready] [medium] Fix and enhance repeat groups. #1939

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

Sophist-UK
Copy link

@Sophist-UK Sophist-UK commented Jan 18, 2018

  1. Fix repeat group min/max functionality where min of zero was not working consistently.

  2. Split Repeat number element into min and max with full backward compatibility

  3. Remove fade of repeat add/remove buttons because not consistent with Bootstrap usage.

  4. Disable add / remove buttons when max/min reached - plus CSS tweaks to make it look better.

  5. Fixing Chosen dropdowns so that the close is reset to as new.

  6. Avoid double resetEvent called from both form.js and inside element.js.

  7. Replicate tooltips correctly. Element tooltips are correctly cloned, add / delete buttons have tips applied and Add tip is no longer cloned if displayed when Add is clicked.

  8. If you allow zero repeat rows and are attempting to add a first repeat row with a validation error, the row is no longer hidden after failed submission.

  9. Elements now get an unload event when you delete a repeat group row.

1. Use -1 as maximum repeat group not set instead of zero in order to distinguish between use of a field to define number of rows which holds zero from maximum not set in Fabrik Group settings.

2. Trigger duplicateGroupsToMin when elements have been added rather
than on timer.

3. Handle reduction correctly in duplicateGroupsToMin.

4. Set min/max in watchRepeatNums not just when field changes.
1.  Move form / details "show group" to be with list / query "show
group" from Layout tab to Details tab. Tweak language strings.

2. Split number of repeats element field into min and max repeat element
fields - in XML, group model, template, JS and language strings.
Backwards compatibility is handled in both front- and back-ends.
... because this is contrary to Bootstrap style and gives the user the
impression that the buttons are disabled.

I have commented this out rather than deleted the code in the event that
users complain and we need to add a group option to enable / disable the
fade.
... depending in min / max repeats allowed.

And tweak CSS for repeat group buttons.
... and to trigget chosen update if needed.

Note: Chosen appears to update already in my tests, but that may simply
be due to how my element JS to make it readonly is sequenced compared to
the JS which chosen-ifies it.
1. It is run explicitly in form.js
2. It is not included in any elements which override this method.
... e.g. if original was readonly or disabled.

NOTE: Chosen update method does not appear to work.
@Sophist-UK
Copy link
Author

Sophist-UK commented Jan 18, 2018

I have tried to work out how to replicate the tooltips when adding a new repeat. The jQuery / tipsBootStrapMock code takes the title attribute off, and it seems impossible to get hold of it to add it to the new cloned version.

I thought about adding Add/Delete group messages as fixed JS strings, which would work because they are fixed, but we could not use this for e.g. validation tooltips which are per element. I also tried to save the title as an HTML attribute on the tooltip bearing tag which will then be cloned with changes in tipBootstrapMock but it wouldn't add it for some reason. So it seems like we need to feed the validation tooltip definitions into the element javascript so that they can be set programatically - but doing tooltips as part of the element js is a fundamental change of design that I am not going to do here.

I will, however, add fixed tooltip text for the add/delete buttons.

1. Stop visible Add tooltip being cloned.
2. Add tooltips to cloned Add/Delete buttons.
@Sophist-UK
Copy link
Author

Sophist-UK commented Jan 18, 2018

This PR is not ready for review.

I need to prevent a new first repeat row being hidden if there are validation errors on it (rather than it being pristine).

@Sophist-UK Sophist-UK changed the title Fix and enhance repeat group min max [medium] Fix and enhance repeat group min max Jan 18, 2018
@Sophist-UK Sophist-UK changed the title [medium] Fix and enhance repeat group min max [medium] Fix and enhance form.js for repeat groups. Jan 19, 2018
... and distinguish between development errors (which are onlu shown
with debug on) and run-time errors which are shown always.
@Sophist-UK
Copy link
Author

Ok - now it is ready for review and merge.

@Sophist-UK Sophist-UK force-pushed the fix_and_enhance_repeat-group_min_max branch from 0b11d63 to 7fb6849 Compare January 19, 2018 21:07
@Sophist-UK Sophist-UK force-pushed the fix_and_enhance_repeat-group_min_max branch 3 times, most recently from fa7f9f5 to 9f40a6b Compare January 20, 2018 21:40
We run a load event when we add a new subgroup (but not when we unhide
it), so we need to run a matching unload event when we delete a subgroup
(but not when we hide it).
@Sophist-UK Sophist-UK force-pushed the fix_and_enhance_repeat-group_min_max branch from 9f40a6b to 5a6064a Compare January 20, 2018 21:51
@Sophist-UK
Copy link
Author

An example of what can be done now we have an unload even on row delete can be found in:

http://fabrikar.com/forums/index.php?wiki/dropdown-selections-unique-in-repeat-group/&noRedirect=1

@Sophist-UK Sophist-UK changed the title [medium] Fix and enhance form.js for repeat groups. [medium] Fix and enhance repeat groups. Jan 20, 2018
...and move multipage setting to the Layout tab, which makes sense.
@Sophist-UK Sophist-UK force-pushed the fix_and_enhance_repeat-group_min_max branch from e7fa52b to 2da09d6 Compare January 22, 2018 00:22
Otherwise if e.g. we have a load event to clear the element, the HTML to clear does not exist and e.g. this.formElements.get(id) in the clear event cannot find the element to clear.
... and implement a clear function.
@Sophist-UK
Copy link
Author

There have been reports of some JS actions not working on some element types, so it might be sensible to add these fixes to this before it is merged.

Needs completion of a dedicated comprehensive test list / form, more testing and probably fixes for further current Fabrik issues once comprehensive testing has identified any other missing functionality.

@Sophist-UK Sophist-UK changed the title [medium] Fix and enhance repeat groups. [notready] [medium] Fix and enhance repeat groups. Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant