mozfet / meteor-autoform-materialize

Meteor AutoForm Materialize templates
https://atmospherejs.com/mozfet/autoform-materialize
MIT License
10 stars 6 forks source link

Nested Arrays and Draggable #70

Open pouya-eghbali opened 5 years ago

pouya-eghbali commented 5 years ago

Hi @mozfet

I'm having issues with nested arrays and draggable. There are a few things that I noticed:

  1. getArrayFromDom doesn't work with nested names (We're doing a split(".") then _.first and _.last ignoring everything between first and last)

  2. Template.afArrayField_materialize.onRendered hook does new Sortable() for each Array element and draggable option is always set to .js-autoform-array-item, this causes conflicts in nested arrays

I'm trying to fix this issue, will report back my findings.

pouya-eghbali commented 5 years ago

Tried doing this, no luck:

  const safeFieldName = fieldName.replace(/\./g, '-dot-')
  instance.$(sortableContainers).find('.js-autoform-array-item').each(function () {
    const alreadyDraggable = instance.$(this).attr('already-draggable');
    if (alreadyDraggable) {
      return
    }
    instance.$(this).attr('already-draggable', 'true');
    instance.$(this).addClass(`draggable-${safeFieldName}`);
  })

  const sortable = new Sortable(sortableContainers, {
    draggable: `.draggable-${safeFieldName}`,
    appendTo: 'body',
    mirror: {
      constrainDimensions: true,
    },
    delay: 250
  })

Problem is, every part of our element can trigger a drag event, won't work with nested draggables (inner draggable fires a drag event for the outer one). Still investigating.

pouya-eghbali commented 5 years ago

I can only find two solutions to this:

  1. Adding a handle so the entire card isn't our handle
  2. not doing new Sortable() if an Array has an already initialized sortable inside

Personally I prefer the first one. Let me know your opinion and/or if you know any other solution to this

mozfet commented 5 years ago

Personally I'm so tired of trying to make nested arrays look good, just wasted another weekend on it. I'm convinced that large draggable arrays are clumsy and look messy and there is nothing we can do to make it better. Its too much data to be dragging around. I want to revisit the whole design approach of arrays in forms and especially nested arrays.

That being said I do not believe it can be done elegantly while showing all the data. We need to show a summary of an array, a one liner a human can work with, drag around, ect: Something clear like:

Screenshot 2019-05-08 at 20 03 11 When you click an edit a modal pop up with a form. That works great for single dimension arrays, but nested would get you into popup overload. So perhaps a split view with a collapsible added on the right as you go deeper into the nest, and pops off whenever you go back...

mozfet commented 5 years ago

I can only find two solutions to this:

  1. Adding a handle so the entire card isn't our handle
  2. not doing new Sortable() if an Array has an already initialized sortable inside

Personally I prefer the first one. Let me know your opinion and/or if you know any other solution to this

A handle seems the easiest... and with such a mass of data, perhaps also the clearest as to what you are dragging.

mozfet commented 5 years ago

Or perhaps a tree with collapsible indented items, like a file structure browser.

mozfet commented 5 years ago

Hi @mozfet

I'm having issues with nested arrays and draggable. There are a few things that I noticed:

  1. getArrayFromDom doesn't work with nested names (We're doing a split(".") then _.first and _.last ignoring everything between first and last)
  2. Template.afArrayField_materialize.onRendered hook does new Sortable() for each Array element and draggable option is always set to .js-autoform-array-item, this causes conflicts in nested arrays

I'm trying to fix this issue, will report back my findings.

Thank you for helping!

pouya-eghbali commented 5 years ago

I'm glad I can help. About making nested arrays look good, my data structure is like this:

Array ( Array ( Object ( String String Number ) ) )

and here's how it looks like:

image

I can solve this by adding some css to my page, but I agree with you, these nested arrays become huge and they look messy. Perhaps we can make another autoform type, for example array-collapsible or something (last item on list is always active/open, item becomes active/open on click, only one item is active at a time). I can also help with this.

About the current issue, I guess it'll not look bad if we add the handle above the delete button. This will work good for nested arrays and arrays of objects, not sure about arrays of strings, numbers or etc, I need to check if their height allows adding the handle above the delete icon.

Another issue we'll be having, is to update all field names of child array items if the parent item is moved, for example all outer.1.inner.4.field-names should become outer.0.inner.4.field-name. This will be a little tricky with the current oldName newName approach.

pouya-eghbali commented 5 years ago

Instead of modifying the current solution and fixing this approach, i'll start one from scratch, if it's OK with you, then if the results are satisfying we can replace the current solution or use some parts of the new solution in the current one.

mozfet commented 5 years ago

Instead of modifying the current solution and fixing this approach, i'll start one from scratch, if it's OK with you, then if the results are satisfying we can replace the current solution or use some parts of the new solution in the current one.

It a YES for me!

I've created a separate enhancement ticket: https://github.com/mozfet/meteor-autoform-materialize/issues/71