Wauw! Treasure trove, really...

[micschk]

I've been keeping a 'cms tweaks' module at hand myself but that's nothing compared to this gigantic collection of improvements. Going through the code in this module, I find a lot of usable stuff, thanks! For example we too often require just save & close, uploads in front-end, various form-related stuff, all great.

Just one remark; I'd like to use only certain parts in a framework-only project (right now mainly filepond, as per your recommendation). Currently this module requires a Page class & pulls in SiteConfig & some more. By default it creates various page types like block/news/contact/etc which is unwanted in this project. We also have our own basic news & FAQ modules, so that'd clash with this module currently. Plus I think 'save & close' is actually getting included into framework by default. Basically in its current state this module seems about 10 various 'Gridfield-enhancements' (all great) combined into one.

I'd like to kindly recommend to split up this module into 'domain-based' smaller modules, optionally with a composer recipe to pull them all in at once when needed (or just a regular composer package which requires all of them).

Domains could be like the headers in the README/namespaces:

• upload-alternatives
• dataobject-enhancements
• admin-enhancements
• modeladmin-enhancements
• cms-contactpage
• cms-faqpages
• cms-newspages
• cms-blockpages
• themable-sites
• subsite-enhancements
• admin-alerts
• social-extensions
• ss-helpers
• etc

We followed this route with our 'gridfield-pages' based modules in SS3, which consist of a reusable tagging (& categorizing) module, a Gridfield 'sitetree-button', exclude-children to hide pages from the sitetree & then some modules for specific purposes which pull them all in, like gridfieldpages, newsgrid, etc. For example the tagging & categorizing module can be re-used on its own which we do often. Also the excludechildren & sitetreebutton modules go installed a lot more than the actual newsgrid module, so apparently those provided some specific functionality usefull beyond the modules we developed them for.

Curious to hear what you think of this suggestion.

[lekoala]

First of all, thank you for your enthusiasm.

Yes I fully agree this module needs splitting! Actually I'm in the process of upgrading a couple of projects to SS4 and this is why I created this module in the first place.

I'm most certainly planning on splitting all this at some point, but I wanted to have a good grasp on the full scope before doing so and dividing things. Some things are not so easily splittable (I'm using helpers all over the place, I need subsite support so that's a tricky thing as well, ...).

Also, currently, since I'm the only one using it, there is not much added value to split things just for the sake of it :-)
If you identify some areas that are useful to you as well and I you like to join the development effort, I'd be more than happy to start splitting this module. I like your proposal to have a "framework only" module.

[micschk]

I've been working on extracting a set of classes concerning (temporary) files & uploads (FilePond working well for back & front-end), and made some tweaks.

Basically I've changed some of the 'Base...' classnames to be more descriptive (like AbstractFileUploadField instead of BaseFileUploadField).

For some others, I've removed the 'Base' part and renamed them to be the same as the classes/traits they replace/extend, just in a different namespace. eg;

namespace LeKoala\SilverStripe\Assets;

// Formerly \LeKoala\Base\Forms\BaseUpload
class Upload extends \SilverStripe\Assets\Upload
{ ...

and:

namespace LeKoala\SilverStripe\Forms;

use LeKoala\SilverStripe\Assets\Upload;

/**
 * Trait FileUploadReceiver
 * Adds better default descriptions and a better default uploadfolder to uploadfields
 * formerly LeKoala\Base\Forms\BaseFileUploadReceiver
 *
 * trait UploadReceiver -> for a formfield which has an Upload() instance and can upload to a folder (no auto-saving into DO)
 * class FileField (uses UploadReceiver implements FileHandleField) -> formfield which auto-saves into has-one on dataobject (->upload->loadIntoFile())
 * trait FileUploadReceiver (uses UploadReceiver, calls constructUploadReceiver() from it) -> operations for writing uploaded files to DO's & into relations (also many_many etc)
 */

trait FileUploadReceiver
{
    use \SilverStripe\Forms\FileUploadReceiver {
        // alias constructUploadReceiver to overriddenConstructUploadReceiver
        \SilverStripe\Forms\FileUploadReceiver::constructUploadReceiver as overriddenConstructUploadReceiver;
    }

    /**
     * Override \SilverStripe\Assets\Upload with LeKoala\SilverStripe\Assets\Upload
     */
    protected function constructUploadReceiver()
    {
        $this->overriddenConstructUploadReceiver();

        // Change Upload instance to be a LeKoala\SilverStripe\Assets\Upload
        $this->setUpload(Upload::create());
    }
    ...

I'd like to get your opinion on some of these changes if that's OK with you;

• As logic for namespaces I tend to use the existing namespace prefixed with my own (eg 'SilverStripe\Assets' becomes 'Restruct\SilverStripe\Assets'), would you consider that OK?
• Does it make sense to have overriding classes/traits use the same classname in a different namespace or do you foresee that causing issues later on? I think it may work well because that seems to be what namespaces are for, and it allows you to swap a class by just changing the imported namespace without changing all classname occurrences in your code).
• Do you think the way I made FileUploadReceiver 'extend' \SilverStripe\Forms\FileUploadReceiver simplifies things or would you consider it a sort of hack instead? (traits cannot actually extend other traits).

[lekoala]

so sorry I didn't get the opportunity to write a proper answer to your proposal.

But here it is!

• For the namespace, I'd rather keep it simple. No need to have SilverStripe each time in it, my SilverStripe modules are not meant to be used anyway with any other php package I could produce :-)
• For all class names, I tend to have "unique" class name. It's such a pain already to have these seemingly simple class names (like... Controller, which always import by default the GraphQL controller instead of the Control one...), but it the end, you end up using aliases, it makes search/replace more difficult... so yeah, now I have more verbose class name, regardless of the namespace.
• For the trait, I'm more up for simplicity but I guess both approaches have their advantages