apostrophecms / apostrophe

A full-featured, open-source content management framework built with Node.js that empowers organizations by combining in-context editing and headless architecture in a full-stack JS environment.
https://apostrophecms.com
Other
4.36k stars 590 forks source link

On-page, in-context editing of areas nested in array elements does not save #993

Closed boutell closed 6 years ago

boutell commented 7 years ago

This turns out to be a longstanding issue:

If I have an area inside a field of type array and I want to edit that area contextually on the page, I can't.

I hoped to fix this simply, but there's a problem. The contextualConvert mechanism depends on scoping the search for relevant elements via a parent element, and array items don't necessarily have one on the front end of websites, or it's site-dependent.

We could address it by saying something ilke this:

"If you want contextual editing of the content of the array elements, you have to introduce a [data-apos-array-item="item._id"] attribute on a wrapper element. You don't have to do this, but you can't have contextual editing unless you do."

This would require minimum change to the code.

Or, we could address it by locating contextually editable fields by their full dot paths, which would mean it wouldn't matter if the intervening structures had a representation in the DOM or not. This would involve tricky fallbacks for bc and could complicate things if someone actually introduces array editing in context (adding/removing/etc).

The first solution makes the most sense to me. We could provide a helper to make it less awkward. People are going to forget to do it and complain. Hmm.

cc @stuartromanek

boutell commented 7 years ago

If such areas appear editable by default when contextual: true isn't even set, then that's arguably a bug to be fixed in the meantime.

stuartromanek commented 7 years ago

Not a fan of option 1, any fix that isn't automatically able to do the things feels like we're balking on the really good stuff at the intersection of 'in-context editing!' and 'really powerful schemas!'.

@boutell are you able to estimate time/difficulty for the One and Truly Blessed fix? We don't have to implement it immediately but its something we should move along the Roadmap.

boutell commented 7 years ago

A day? Like a real actual day though

boutell commented 7 years ago

I think that contextual editing of the array itself, as in adding and removing things, is something that is likely to eventually happen. And this would require some structure always be present in the DOM so we can find the array items as such, not just make inferences about areas within them. So it would make sense to introduce a helper for outputting array items, with the HTML element and other attributes of your choice of course.

However another good workaround for this issue is to use areas rather than arrays. Define a widget for the items, and use areas containing only those items. Works out to much the same thing and it works completely contextually.

You might not be as big a fan of this workaround if you're building something like a tabs widget, as it would force you to not do your fancy javascript for showing and hiding tabs when an editor is logged in.

kolbma commented 6 years ago

Well the editing in context works here with latest stable. But it doesn't get saved. It stays empty after reload. On the other side in the edit overlay I can't add widgets to an area. The +-menu is empty.

boutell commented 6 years ago

Yes this is pretty much what we see. I've updated the title to clarify.

However are you saying it doesn't work in the modal dialog box either? That would be new.

bionara commented 6 years ago

Any update on this? I have just noticed the bug myself: https://stackoverflow.com/questions/48123200/apostrophe-cms-inline-editing-of-singleton-area-in-custom-widgets-html-not-w

bionara commented 6 years ago

I've updated that stackoverflow comment with some more info. I think the issue is that the singleton/areas embedded in the widget do not have their ID committed to the DB yet, which the lock method requires as part of its save process?

bionara commented 6 years ago

Is there a known workaround for this? Or is there some way I can rig up a direct save to mongo eg on field blur? I assumed I could do this by sending the _id off and creating a new record if it doesn;t exist?

boutell commented 6 years ago

The known workaround is to use nested areas rather than the array schema field type. It achieves the same goal and is actually more contextual, although I can imagine scenarios where it might not be suitable.

boutell commented 6 years ago

(Actually, clearly describing one of those scenarios might go some way toward getting this implemented.)

If you're having trouble of this kind that doesn't involve an array schema field, that would be a separate ticket and should be opened as such with a complete example that reproduces the problem.

bionara commented 6 years ago

Thanks @boutell ! I have to admit that I don't know if I am using array schema field type or not sorry. Here's a summary of what I am doing:

home.html has an area with the option to use my custom widget multi-cols. Multi-cols opens the modal and lets the user specify col number. The widget is rendered with a header singleton, and then a loop to create X columns, each having its own singleton.

In order to exclude possible causes for the bug, I tested this by deleting all code in the custom widget other than the simple header singleton, but it still does not save. Here's how the code looked in that case (ie simplified to eliminate other factors):

home.html:

{{ apos.area(data.page, 'homeBlocks', {
  widgets: {
    'multi-col': {}
   }
 })
 }}

multi col index.js:

module.exports = {
  extend: 'apostrophe-widgets',
  label: 'Multi Column Layout',

  addFields: [
    {
        type: 'select',
        name: 'numberOfColumns',
        label: 'Number of columns',
        choices: [
            {
              label: '1',
              value: '1'
            },
            ...etc 
        ],
        required: false
    }
  ]
};

and multi col views/widget.html:

<div class="multi-col-wrapper row">
    <div class="col-xs-12 multi-col__title">
        {{ apos.singleton(data.widget, 'headerText', 'apostrophe-rich-text', {
            addLabel: 'Add header text',
      editLabel: 'Edit header text',
        toolbar: [ 'Styles', 'Bold', 'Italic', 'Link', 'Unlink', 'BulletedList' ],
        styles: [
          { name: 'Title', element: 'h2' },
          { name: 'Subtitle', element: 'h3' }
        ]
      }) }}
    </div>
</div>

Please let me know if you need any more info/code snippets as I am keen to get this nailed :)

bionara commented 6 years ago

@boutell would you mind explaining or linking to docs for how to achieve this: :"The known workaround is to use nested areas rather than the array schema field type". I'm not quite sure how to go about it. Thanks for the help!

bionara commented 6 years ago

I just tried this on another apostrophe project I have which does not use workflow and the bug persists

boutell commented 6 years ago

For the record, #1182 is a separate and unrelated issue and is not a bug. Spontaneous areas (areas that don't get defined in their schema) are allowed at the root level only. The use case @bionara had in mind can be achieved by creating several widget types, one column, 2 column, etc., which define the needed areas in their schemas as well as outputting them in widget.html.

boutell commented 6 years ago

Oops, didn't mean to close the original issue re: contextual editing of fields of type array with areas in them.

bionara commented 6 years ago

Cheers! I figured a work around: put the fields (as named in widget index.html) into the index.js schema. The multi col stuff I have a select for 1/2/3/4, and then in the template do an "if X then render Y areas" and it all works.

One further question - how can i hide items from the widget modal? Ie I don't want these ones to show in the modal, but in context only. I thought I could get away with this simple override but wasn't so lucky:

arrangeFields: [
    {
      name: 'info',
      label: 'Info',
      fields: ['allOfMyFieldsThatIWant']
    }
   ]
boutell commented 6 years ago

Oh yes, that would also work, if you don't like the separate widget types as choices the user is making earlier. Good idea.

Use contextual: true for the fields that should not be editable/visible in the modal.

bionara commented 6 years ago

Thanks @boutell ! Out of interest, would it be worth submitting simple widgets such as multi col one back to the apos community? IF so, how do you do that?

boutell commented 6 years ago

That's a good idea. Yes, here are guidelines and tips on how to do that:

http://apostrophecms.org/docs/more-modules.html#publishing-your-own-npm-modules-for-apostrophe

boutell commented 6 years ago

This PR will close this issue when merged:

https://github.com/apostrophecms/apostrophe/pull/1656

There are some more esoteric cases it doesn't cover: areas nested in arrays nested in arrays, for instance, with no intervening widget. We can address that at some point if it's really necessary. But this is probably as far as we will take it before 3.0 and the rebuild of contextual editing.

beaulac commented 5 years ago

This bug is back as of 2.71.0 ; the last working release is 2.70.1.

It seems the work done in #1656 was removed in this commit:

Nothing in the changelogs or commit messages seem to mention this; @boutell was this intentional?

boutell commented 5 years ago

Definitely not intentional, good catch, thank you. I am bringing this fix back.

On Fri, Nov 30, 2018 at 2:19 PM antoine beauᵛᵃᴵˢ⁻lacᵃˢˢᵉ < notifications@github.com> wrote:

This bug is back as of 2.71.0 ; the last working release is 2.70.1.

It seems the work done in #1656 https://github.com/apostrophecms/apostrophe/pull/1656 was removed in this commit https://github.com/apostrophecms/apostrophe/commit/c77793abaccb54c7b5c0ead5f46d4b96555d450e#diff-b0d621dfdf98ca7597f35de72d06395d :

Nothing in the changelogs or commit messages seem to mention this; @boutell https://github.com/boutell was this intentional?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-443310275, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fSPiVa9Ft4k-dzlVIyb5DASVB_phks5u0YTMgaJpZM4PJK9w .

--

Thomas Boutell, Chief Software Architect P'unk Avenue | (215) 755-1330 | punkave.com

staminna commented 3 years ago

Is this functionality lost in version 3.x branch? I am looking for the Drag and Drop reordering of pictures and I can't seem to find it.

boutell commented 3 years ago

Hi Jorge,

In 3.x we went with the new cut and paste functionality, please give that a try. It is actually much easier to work with. Dragging over long distances is awkward, and it was not possible between pages. Both cases work great with the cut and paste feature (see the scissor icon after clicking on a widget; then go to the new location, click the + sign, and you'll get an option to paste it there).

staminna commented 3 years ago

Any way to install version 2.x with this funcionality?

A terça, 21/09/2021, 17:59, Tom Boutell @.***> escreveu:

Hi Jorge,

In 3.x we went with the new cut and paste functionality, please give that a try. It is actually much easier to work with. Dragging over long distances is awkward, and it was not possible between pages. Both cases work great with the cut and paste feature (see the scissor icon after clicking on a widget; then go to the new location, click the + sign, and you'll get an option to paste it there).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924177383, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5Y3HUH73I3RJBQU3L6W3UDC2XTANCNFSM4DZEV5YA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

boutell commented 3 years ago

Hi Jorge,

You can use 2.x but keep in mind there will be no major feature additions to that release and support for it will end in a few years. The programming experience is quite different and there won't be any backporting of features like async components, the new module format, etc. I really wouldn't recommend going backwards.

I hear you that you have a strong preference for drag and drop. May I ask if you tried the new cut and paste feature?

On Tue, Sep 21, 2021 at 2:39 PM Jorge @.***> wrote:

Any way to install version 2.x with this funcionality?

A terça, 21/09/2021, 17:59, Tom Boutell @.***> escreveu:

Hi Jorge,

In 3.x we went with the new cut and paste functionality, please give that a try. It is actually much easier to work with. Dragging over long distances is awkward, and it was not possible between pages. Both cases work great with the cut and paste feature (see the scissor icon after clicking on a widget; then go to the new location, click the + sign, and you'll get an option to paste it there).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924177383 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAD5Y3HUH73I3RJBQU3L6W3UDC2XTANCNFSM4DZEV5YA

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924263747, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27N6F6R3TIBXONJKR63UDDGMPANCNFSM4DZEV5YA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell commented 3 years ago

(Bear in mind that you can't take an existing database and codebase back in time to 2.x without a rewrite of both.)

On Tue, Sep 21, 2021 at 3:23 PM Tom Boutell @.***> wrote:

Hi Jorge,

You can use 2.x but keep in mind there will be no major feature additions to that release and support for it will end in a few years. The programming experience is quite different and there won't be any backporting of features like async components, the new module format, etc. I really wouldn't recommend going backwards.

I hear you that you have a strong preference for drag and drop. May I ask if you tried the new cut and paste feature?

On Tue, Sep 21, 2021 at 2:39 PM Jorge @.***> wrote:

Any way to install version 2.x with this funcionality?

A terça, 21/09/2021, 17:59, Tom Boutell @.***> escreveu:

Hi Jorge,

In 3.x we went with the new cut and paste functionality, please give that a try. It is actually much easier to work with. Dragging over long distances is awkward, and it was not possible between pages. Both cases work great with the cut and paste feature (see the scissor icon after clicking on a widget; then go to the new location, click the + sign, and you'll get an option to paste it there).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924177383 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAD5Y3HUH73I3RJBQU3L6W3UDC2XTANCNFSM4DZEV5YA

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924263747, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27N6F6R3TIBXONJKR63UDDGMPANCNFSM4DZEV5YA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

staminna commented 3 years ago

Dear Tom,

I understand, but the copy-paste needs too many clicks when you have hundreds or even thousands of images to upload.

Perhaps we could order them in the image upload view and then they would show up in order on the main page? I don't think that is possible at the moment.

I added a couple of images and I cannot select them. Please check picture on my dropbox:

https://www.dropbox.com/s/h6qsq46m1mnyvil/Screenshot%202021-09-21%20at%2021.03.32.png?dl=0

Also how can I make a three column layout? I can't seem to find that option any longer.

Many thanks Jorge Nunes

boutell commented 3 years ago

Hi Jorge, thanks for clarifying. I see what you mean. It is confusing that you can't reorder the selection on that screen. We're discussing it.

In the meantime though, if you hit save on this screen, you'll discover that you can reorder the selection on the previous screen, where the relationship field is.

On Tue, Sep 21, 2021 at 4:06 PM Jorge @.***> wrote:

Dear Tom,

I understand, but the copy-paste needs too many clicks when you have hundreds or even thousands of images to upload.

Perhaps we could order them in the image upload view and then they would show up in order on the main page? I don't think that is possible at the moment.

I added a couple of images and I cannot select them. Please check picture on my dropbox:

https://www.dropbox.com/s/h6qsq46m1mnyvil/Screenshot%202021-09-21%20at%2021.03.32.png?dl=0

Also how can I make a three column layout? I can't seem to find that option any longer.

Many thanks Jorge Nunes

Tom Boutell @.***> escreveu no dia terça, 21/09/2021 à(s) 20:24:

(Bear in mind that you can't take an existing database and codebase back in time to 2.x without a rewrite of both.)

On Tue, Sep 21, 2021 at 3:23 PM Tom Boutell @.***> wrote:

Hi Jorge,

You can use 2.x but keep in mind there will be no major feature additions to that release and support for it will end in a few years. The programming experience is quite different and there won't be any backporting of features like async components, the new module format, etc. I really wouldn't recommend going backwards.

I hear you that you have a strong preference for drag and drop. May I ask if you tried the new cut and paste feature?

On Tue, Sep 21, 2021 at 2:39 PM Jorge @.***> wrote:

Any way to install version 2.x with this funcionality?

A terça, 21/09/2021, 17:59, Tom Boutell @.***> escreveu:

Hi Jorge,

In 3.x we went with the new cut and paste functionality, please give that a try. It is actually much easier to work with. Dragging over long distances is awkward, and it was not possible between pages. Both cases work great with the cut and paste feature (see the scissor icon after clicking on a widget; then go to the new location, click the + sign, and you'll get an option to paste it there).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924177383

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5Y3HUH73I3RJBQU3L6W3UDC2XTANCNFSM4DZEV5YA

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <

https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924263747

,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAAH27N6F6R3TIBXONJKR63UDDGMPANCNFSM4DZEV5YA

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924314418 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAD5Y3BQGLMZZ2LRVT5FNJTUDDLXHANCNFSM4DZEV5YA

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924343380, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27MVNPEQDXLJR3D7SJDUDDQTNANCNFSM4DZEV5YA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

stuartromanek commented 3 years ago

Hi @staminna , what Tom means is that items joined in a relationship (images in this case) can be re-ordered via the relationship field UI.

The example you're looking at right now (the image widget in the demo) is set to only allow a single image to be selected (max: 1), so you won't be able to save your multi image selection and reorder them on the next screen in this example. If you wanted to create a slideshow widget where you could select multiple images and reorder them, you'd create a new relationship field without a max (or a max set to a higher number).

To be clear, here is reordering items in a relationship

https://user-images.githubusercontent.com/1889830/134243332-6852e294-3795-47e9-a2a1-c0060547c9d2.mov

boutell commented 3 years ago

I believe @staminna sent a screenshot that featured multiple images selected, so I assumed he had a custom relationship going with no "max: 1" and noticed this when he hit "browse," but Jorge you can correct me if I'm wrong about that.

On Tue, Sep 21, 2021 at 4:34 PM Stuart Romanek @.***> wrote:

Hi @staminna https://github.com/staminna , what Tom means is that items joined in a relationship (images in this case) can be re-ordered via the relationship field UI.

The example you're looking at right now (the image widget in the demo) is set to only allow a single image to be selected (max: 1), so you won't be able to save your multi image selection and reorder them on the next screen in this example. If you wanted to create a slideshow widget where you could select multiple images and reorder them, you'd create a new relationship field without a max (or a max set to a higher number).

To be clear, here is reordering items in a relationship

https://user-images.githubusercontent.com/1889830/134243332-6852e294-3795-47e9-a2a1-c0060547c9d2.mov

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/993#issuecomment-924362856, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OIKRPMV525LV2EGRTUDDT4FANCNFSM4DZEV5YA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

stuartromanek commented 3 years ago

@boutell he did, but notice the disabled Select/Save button in it

boutell commented 3 years ago

Good point Stu.

staminna commented 3 years ago

I can't find the button to save

staminna commented 3 years ago

Edit: I created a relationship field with a max of 10.

I only changed the secret and app name.

Strange I can't see the Save button.