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

Adding a new field removes previous labels #354

Closed
jakedaleweb opened this issue Sep 23, 2015 · 14 comments
Closed

Adding a new field removes previous labels #354

jakedaleweb opened this issue Sep 23, 2015 · 14 comments

Comments

@jakedaleweb
Copy link

jakedaleweb commented Sep 23, 2015

When adding a new field in user forms 3.0 if you have any unsaved fields with a label the label is removed when the new field is added.

all of these are unsaved labels
screen shot 2015-09-24 at 11 01 03 am

add new field is pressed
screen shot 2015-09-24 at 11 40 56 am

all labels are set to blank
screen shot 2015-09-24 at 11 41 00 am

ACs

  • Field labels are retained on the form after a new field has been added
  • Field types are retained on the form after a new field has been added
  • Page names are retained on the form after a new field has been added
  • Field group, and any other editable fields are retained on the form after a new field has been added

Implementation notes

  • Ideally the full form does't need to save when every new field is added - that's going to result in long load times for basic form editing tasks,

PRs

@dallumnz
Copy link

Hello,
Are there any plans to resolve this issue? It seems to be continuing with the latest versions of the framework and CMS.

@tractorcow
Copy link
Contributor

Would it be ok if the labels were saved?

@dallumnz
Copy link

That would be great, thanks.

@tractorcow
Copy link
Contributor

tractorcow commented Apr 29, 2016

ok, then maybe what we should do is make "add field" also save the underlying list before reloading it. It's on draft so it should be ok to write the changes to stage.

@dasplan
Copy link

dasplan commented Sep 5, 2016

Hi, I need this feature for a client. Any idea where to fix this, where to save the labels?
edit:
I'm currently saving ClassName and Title in FieldEditor.js for each field on add-button click and restoring it in the one('reload') function in the 'addnewline' event handler.

@clarkepaul
Copy link

A duplicate issue to this one was raised more recently. Some of the discussion maybe of use to resolving this issue e.g. using local storage, showing a dialog to say you have unsaved changes... and some more.
#757

@a2nt
Copy link

a2nt commented Jul 9, 2020

I think field values can be stored stored globally by JS into browser's Local Storage
the field values will be set to fields on refresh and the storage values will be flushed on Save or Save & Publish

@emteknetnz
Copy link
Member

emteknetnz commented Aug 12, 2021

There's currently a browser dialog that's triggered in firefox and chromium when you attempt to navigate away from the page by clicking a link, such as a form field edit link, or a page in the site tree.

image

Seems like it's just "Add field" / "Add Page Break" / "Add Field Group" buttons that allow an action to happen without triggering the browser dialog warning on unsaved changes

@brynwhyman
Copy link

Seems like it's just "Add field" / "Add Page Break" / "Add Field Group" buttons that allow an action to happen without triggering the browser dialog warning on unsaved changes

Arguably that fixes the bug, but I have a feeling that almost makes the experience worse. To start with, the user isn't "navigating away from the page" so the browser dialog is (at least the first time you read it) very confusing. Plus I'd expect a common flow is to want to bulk add a couple of field in one go before saving the page.

If there's ideas against local browser storage then it could be worth reconsidering saving the page when a user selects one of the "Add" fields. 19 fields have reported to take 10 seconds to save which is a lot of waiting when creating a form.

@GuySartorelli
Copy link
Member

GuySartorelli commented May 13, 2022

I can think of three way to resolve this:

  1. Change the behaviour when you click one of the add buttons - instead of creating the new DataObject and relation, it would just add a field on the front-end. The underlying objects wouldn't be created until you click save.
  • This would be a departure from the current behaviour
  • This could result in timeouts or running out of memory if users attempt to add too many fields in a single save
  1. Don't allow any inline-editing of the fields.
  • This is an even larger departure from the current behaviour
  • Arguably worse UX
  1. Go the elemental route and turn this into a react-with-graphql interaction.
  • A lot of work
  • Keeps the same behaviour of creating actual objects and relations when you click add
  • Keeps inline editing
  • Probably breaks some custom javascript people have added for their own projects

@emteknetnz
Copy link
Member

emteknetnz commented May 18, 2022

Change the behaviour when you click one of the add buttons - instead of creating the new DataObject and relation, it would just add a field on the front-end.

There's a class of bug in the CMS, which is particularly noticeable in asset-admin when navigating around after adding a new file/folder, where there are 2 sources of truth - what's stored in memory in the front-end, and reality i.e the database. I'd like to avoid 2 sources of truth as much as possible.

Also this just seems like it won't work - what happens when you create a new field and immediately click the edit button?

Don't allow any inline-editing of the fields

I'd say this is the best of the 3 suggestions, though it's still not great. Still show the label which can be edited on the non-inline editable, just disable inline-edit-ability to prevent data loss

Go the elemental route and turn this into a react-with-graphql interaction

I'm even less keen on this right now, the elemental 'form-within-a-form' approach created a whole new class of issues, such as a very inconsistent validation handling. Have a weird hybrid with both a "sideways" xhr form inside a regular silverstripe form behaviour is fragile architecture, at least as it stands now where the "sidways" form was kind of just bolted on top of the existing form.

This is however the "correct" solution to inline editing. It feels like Silverstripe just needs a more mature architecture to properly facilitate inline editing that can be used in a variety of places. It may involve ditching the traditional form architecture entirely

That leaves the other 2 suggestions, neither of which seems particularly great

  • Alert message when navigation away (poor/weird ux)
  • Local storage to store the field titles - this is a 2nd source of truth when isn't good. What happens when you inline edit the field, don't click the save button, and then click the edit button, modify the title in the non-inline form and save that? This solution just feels like it's going to introduce syncing bugs, just like asset-admin has.

If I had to choose one right now, personally I'd just disable inline-editing. This is a loss of functionality though, so we're probably best of just leaving this as is for now.

@purplespider
Copy link
Contributor

I'd say this is the best of the 3 suggestions, though it's still not great. Still show the label which can be edited on the non-inline editable, just disable inline-edit-ability to prevent data loss

If I had to choose one right now, personally I'd just disable inline-editing. This is a loss of functionality though, so we're probably best of just leaving this as is for now.

Agreed. Even if we did this for the short term, I would still like to see a longer-term plan to provide the best UX possible in this area. However, I get your point about Silverstripe needing a more mature architecture to facilitate this.

Another issue with just disabling inline editing: The only way you can currently set/change the field type is via the inline editing. You can't change the type when editing it not inline. (Presumably, because the editable fields vary depending on the type, and there isn't a built-in way to instantly change which fields are visible). So we'd still need to find a solution to this if removing the inline editing completely.


Is there really no way to get it to inject the newly created field into the bottom of the list, rather than having to refresh the whole list (which is what loses unsaved inline changes)?

@emteknetnz
Copy link
Member

emteknetnz commented May 20, 2022

Turns out the existing architecture is actually good enough to do what we want. When you click the 'Add Field' button it actually submits the existing form data to a gridfield endpoint, it's just we weren't handling the modified form data.

I've created a pull-request to save the modified values.

I've tested that both Title and ClassName are able to be changed on add

@emteknetnz emteknetnz removed their assignment May 20, 2022
@GuySartorelli GuySartorelli removed their assignment May 23, 2022
@emteknetnz
Copy link
Member

Linked PR has been merged, will be released as part of userforms 5.14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests