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

Feature/466 autosave #485

Draft
wants to merge 2 commits into
base: 2.x
Choose a base branch
from
Draft

Feature/466 autosave #485

wants to merge 2 commits into from

Conversation

finnlewis
Copy link
Member

@keelanfh @markconroy @andybroomfield

Here's a PR to test, just adds and installs the module with default configuration (saves content forms every 60 seconds)

Coming from #466

@finnlewis finnlewis linked an issue Jan 11, 2023 that may be closed by this pull request
@andybroomfield
Copy link
Contributor

Initial look through.
Installing this branch does not appear to enable autosave by default in an existing install.
Once enabled, it can still be removed.
I noticed it doesn't come with any configuration so to be useful is going to need to be enabled and set to autosave on all content types, which I guess also means there is going to be something needed to update that, which brings back the issues in the original PR about how to make this opt in.

@keelanfh
Copy link
Member

@andybroomfield If you don't select any of the content types, it defaults to being enabled for all content types. So I don't think that would require ongoing changes.

@markconroy
Copy link
Member

Would be good to have a notification or something to existing sites via drupal_set_message to say "Hey, we've added an autosave module, would you like to enable it".

We've lots of nice new features that existing sites don't get by default. Sorry, I know this is a side issue to the main PR.

@finnlewis
Copy link
Member Author

No need to apologise Mark, this was a large topic of conversation in our Tech Group Governance meeting yesterday.

Specifically: "What is the policy for adding modules to default install LocalGov eg. autosave"

I will draft a policy for review, but key questions include:

  1. are there at least 2 councils requesting a new module or feature?
  2. Are users able to optionally disable the module / feature?
  3. Are most of our users using / wanting the module / feature?
  4. Should we check with the product group / product owner?
  5. How do we communicate new potential additions to existing instalations? (Release notes / README / drupal_set_message like you suggest @markconroy ?)

So perhaps we can use this pull request as a case in point to draft and agree a working policy for such things.

@andybroomfield
Copy link
Contributor

Thanks @keelanfh I did notice that after a while, I got thrown as autosave does not work for /node/add, so only noticed it when I went back to edit.
I'd worry though about rolling it out to all content, to be useful it may welll need to have some default config shipped with it, which brings us back to how to make this opt in.

@finnlewis finnlewis mentioned this pull request Jan 12, 2023
@andybroomfield
Copy link
Contributor

Worth putting this to draft whilst the discussion is ongoing @finnlewis ?

@finnlewis finnlewis marked this pull request as draft January 16, 2023 14:22
@finnlewis
Copy link
Member Author

I've started a draft policy for discussion on the general questions this raises: https://github.com/localgovdrupal/localgov/wiki/%5BDraft%5D-Policy-for-adding-a-new-feature,-functionality-or-modules-to-LocalGov-Drupal

@keelanfh
Copy link
Member

@andybroomfield that's interesting that it doesn't work for /node/add, I was not aware of that.

@andybroomfield
Copy link
Contributor

Going to need at least these patches on core I think to get it to work with paragraphs
https://www.drupal.org/project/drupal/issues/2824097
https://www.drupal.org/project/drupal/issues/2844229

And this patch on the module to get autosave on node creation
https://www.drupal.org/project/autosave_form/issues/2924006

@finnlewis
Copy link
Member Author

This is still being discussed in the backlog group with some updates being tested.

@finnlewis
Copy link
Member Author

Still being discussed #466

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.

Autosave content
4 participants