silverstripe / recipe-cms

Silverstripe recipe for fully featured page and asset content editing
BSD 3-Clause "New" or "Revised" License
20 stars 14 forks source link

Add new login forms module to recipe #26

Closed chillu closed 4 years ago

chillu commented 5 years ago

It's a requirement of installing the MFA module, which we might make a part of our default recipe eventually. But until then, the restyled login forms themselves provide great value as part of the default recipe: https://github.com/silverstripe/silverstripe-login-forms

Ideally we find a way to only install them in new 4.x installs, and add upgrade instructions for how to opt-in to them for upgrading projects.

Pull Requests

chillu commented 5 years ago

OK, I'm targeting 4.5.0 with this, assuming that roughly coincides with getting the module out in a CWP recipe (late August?).

I don't think that necessarily means a stable release of silverstripe/login-forms (it's currently in beta), unless we want to push for that for the associated CWP recipe release. The reality is that the module isn't battle tested. I wouldn't be surprised if we needed to add more customisation abilities. The PHP and YAML "API surface" of this module is so small that we could still call it stable though.

Pulling in 4.0@beta now means that we can get more people testing this, and we'll need to revisit if we can remove the beta constraint by the time 4.5.0 comes out.

In terms of inclusion, inlining it in installer means it only applies to new projects. Adding it to a recipe will make it opt-out rather than opt-in for many people. Are there any downsides or maintenance issues I'm missing with this approach?

Note that the module is already commercially supported by SS Ltd, but becomes a bit more of a "core team maintenance concern" by including it in installer. Ideally we'd have broader recipes than just the (bare-bones) installer, but professional looking login forms are a baseline for me - it's literally the first touch point of users with the CMS, and looked a bit shit in about 2/3 of all SIlverStripe installs I've interacted with over the years.

/cc @silverstripe/creative-commoners @silverstripe/core-team @silverstripeux

ScopeyNZ commented 5 years ago

I would be pretty happy tagging a stable before it's "battle tested". I think we can be confident enough with the release of our module to do that. It definitely should be at least RC before it makes it into a recipe release imo.

brynwhyman commented 4 years ago

We didn't get silverstripe/silverstripe-installer#257 merged before 4.5-rc1 was tagged. We should make sure to get this in so it's part of new sites for 4.6+

robbieaverill commented 4 years ago

PRs merged