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

Stop replacing the default Content field #410

Closed
NightJar opened this issue Sep 25, 2018 · 19 comments · Fixed by #732
Closed

Stop replacing the default Content field #410

NightJar opened this issue Sep 25, 2018 · 19 comments · Fixed by #732

Comments

@NightJar
Copy link
Contributor

https://github.com/dnadesign/silverstripe-elemental/blob/265b9481af13c2eb34f817bb3308ba937eeb634d/src/Extensions/ElementalAreasExtension.php#L151

This line is no longer necessary, and produces unexpected behaviour when someone wishes to have both a Content field (which may not necessarily be an HTMLEditorField) on a Page (or DataObject) that has Elemental applied.

This seems to me to be a hang-over from the way things used to be... but now Elemental is more versatile and can be applied in many more situations than to pre-existing pages... I think this can be done away with - or at least moved to into another extension.

@robbieaverill
Copy link
Contributor

Can you explain a little about why you've classified this as high impact?

The context around the line is that elemental should replace the default page content area with content blocks. If you remove this line then you'll have a content area again, which doesn't work with the blocks flow.

We could maybe make the removal of Content configurable, so if someone wants to use their own Content field as well as blocks then they could - would that suffice?

@NightJar
Copy link
Contributor Author

NightJar commented Sep 27, 2018

The impact is high because it (re)adds in a feature that people wouldn't expect and would have to deal with upon an upgrade.
Although the field is there always, it is replaced by default - feedback I've heard recently (#content-blocks, community slack) has indicated that this is often contrary to expectation, and people would like the content field back (for whatever reason - it is not our place to judge).

The outcome would be that the extension would only add fields, leaving it up to the project author to decide whether or not any other fields should also be hidden/replaced/removed.

This is an issue instead of a PR exactly for this discussion :)

My understanding is that the replacement is for fields from other extensions that reference it as the "insertBefore" parameter to adding fields to the list, otherwise it would have been removed.
My understanding for why it would be removed is that it was relied upon in the past for things such as searching, and was intended to not have user input placed in it. However as this is no longer the case, this field is open to be used again in the manner originally intended by Page... except that the current status is for Elemental to impose a decree that it shall not be used. I don't think Elemental should be that narrow minded as to think it is the solution to every problem... as by that way we should remove Title as well and create a block type for it ;)

@robbieaverill
Copy link
Contributor

feedback I've heard recently (#content-blocks, community slack) has indicated that this is often contrary to expectation, and people would like the content field back

If we make it configurable then this could be achieved easily

The outcome would be that the extension would only add fields, leaving it up to the project author to decide whether or not any other fields should also be hidden/replaced/removed.

Sure, but we don't want a content field displayed when we're in an elemental page

My understanding is that the replacement is for fields from other extensions that reference it as the "insertBefore" parameter to adding fields to the list, otherwise it would have been removed.

Yeah I think that's a fair point, however devs probably shouldn't be relying on fields existing or not since modules can do whatever they want to CMS fields (like this module does)

I don't think Elemental should be that narrow minded as to think it is the solution to every problem... as by that way we should remove Title as well and create a block type for it ;)

Do you have another solution? If you add the content field back it's going to break our UI patterns for elemental but I'm open to suggestions.

The title field is not part of the page's content, and is used as structured data for many other things than rendering a page's content on the frontend. I think moving Title into a block would be a mistake and would add a huge amount of complication and extra complexity to how SilverStripe works

@robbieaverill
Copy link
Contributor

As an aside: this behaviour is intended, so we can't really call this a bug. I'm happy for you to retarget this as an enhancement and make the content field handling configurable though

@soulroll
Copy link

soulroll commented Sep 28, 2018

Making it configurable via yml would be an ideal solution. If you are looking for a bug then there is no way to remove "You can fill this page out with your own content, or delete it and create your own pages." generated with the default pages (About and Contact) on a fresh SilverStripe Install without removing the pages.

@NightJar
Copy link
Contributor Author

NightJar commented Sep 28, 2018

@robbieaverill It is labelled as an enhancement :>

is used as structured data for many other things than rendering a page's content on the frontend.

A fair comment. It was a bad example.

Sure, but we don't want a content field displayed when we're in an elemental page

Don't we? Elemental upon installation does not apply itself to any pages, and we even create a special type of page in recipes to support it separately from other page types. So maybe we don't but I've heard from several community developers wishing to add the Content field back in to page types they've added the Elemental extensions to. It is certainly not a hard task to hide the Content field if one so desires.

If we make it configurable then this could be achieved easily

Yes a feature flag style alteration would be one way to solve this issue.
e.g.

private static $extensions = [ElementalPageExtension::class];
private static $elemental_please_dont_hide_my_content_field = true;

There are other ways that could see us merge this in a minor patch though, such as separate extensions (although that could introduce confusion around which to apply), or a proxy that applies leverages the current extension then also removes the content field (probably too complex a solution).

@soulroll You can run the dev/build with a flag (GET var) that disables the requireDefaultRecords section. Not ideal if you want some of the others though...

@robbieaverill
Copy link
Contributor

Can we just make the field removal/replacement configurable?

@phptek
Copy link

phptek commented May 1, 2019

@robbieaverill

Sure, but we don't want a content field displayed when we're in an elemental page

Every site I've worked on with Elemental, the client wants a Content field and a list of blocks. Some want the blocks above Content, others want it below.

I've never understood the insistance on a page exclusivley being for Blocks or Content. Why are they mutually exclusive? (Hint: They're not!).

@robbieaverill
Copy link
Contributor

I guess I would've thought that a content field is simply another block. That may have been my mistake for assuming that though. I know that the silverstripe-blocks module adds blocks around the default content area, whereas elemental has traditionally replaced it. Seems like people want it to be configurable though (based on the reactions to my previous comment), so I guess that's the way forward.

@ScopeyNZ
Copy link
Member

ScopeyNZ commented May 1, 2019

@bcairns
Copy link

bcairns commented May 22, 2019

I would like this ability too.

I think users should be able to just create a new page and start typing content. Adding blocks should be optional "advanced functionality" for page building, not a replacement for the simple default workflow.

And what if you want to add Elemental to a bunch of existing pages that already have Content populated? "Sorry, recreate everything with content blocks"?

Elemental should not be making this decision for me.

@robbieaverill
Copy link
Contributor

Side note:

And what if you want to add Elemental to a bunch of existing pages that already have Content populated? "Sorry, recreate everything with content blocks"?

There's a content migration task for this, see the previous comment from ScopeyNZ

@TheBnl
Copy link

TheBnl commented Sep 19, 2019

For people looking to re-insert the content editor, i've created a module that inserts the field and adds a "page content block" by default so any content that exists on a page gets rendered with the added flexibility of using the layout from Elemental.

I think a "page content block" is too much of a stray of the core module, but do agree that removing the content editor could be made configurable as @NightJar suggested.

@brynwhyman
Copy link

@TheBnl thanks for sharing. Have you thought about sharing this on the #content-block community slack channel or the SilverStripe forum? I think the problem you're trying to solve might start some interesting discussions (as I see this issue is now 1 year old).

I think that content blocks are a relatively simple workflow for editors to 'just start adding content' as the default editing experience, so it's interesting to see other's take on it. If this is made configurable, what would you see the default being?

@NightJar
Copy link
Contributor Author

Update since we're all here talking - @ichaber has an open PR to add a configuration option.
It needs rebasing, but other than that should add the configuration option without issue.

The only thing I noted was that it will do it globally for all pages in all areas. If one would like a more controllable option with a per area setting, then this would require a bit more work.

Does anyone have a strong preference for the "per page" option of whether or not the default $Content area remains?
Otherwise I'm going to merge and it'll remain global until another issue is opened :)

@robbieaverill
Copy link
Contributor

robbieaverill commented Sep 19, 2019

Per page would be nice, but probably not essential. Devs can still remove it again at a page level if they want to re-enable it globally but turn it off for certain page types.

The recommended use case for elemental is that you don't have the default content field in place, that's unlikely to change going forward.

@NightJar
Copy link
Contributor Author

OK, so I updated the PR, and after some light thought it introduces config for applied classes to override the global setting too - this was an easy change although the API isn't "solid" it is in line with SilverStripe conventions.
Current defaults are maintained - the content field will be replaced unless config values are present.

@TheBnl
Copy link

TheBnl commented Sep 20, 2019

I think the need for these sort of hybrid solutions arises the most on pages like Blog posts. Those pages follow a consistent structure but sometimes extend the content with a video or gallery block.

What the default should be also depends on the type of project that you're working on. For example a marketing site needs the flexibility of tailored pages for all page types but a company site needs the flexibility on the home page but not on the underlying detail pages.

The problem with a solution like I made is that you get in this weird space of editing some parts on the Page model and other parts in the Elemental area. Making it unclear where and when to edit.

The best would be one coherent editing experience, preferably in the blocks interface. Ideally when a page is created the editor is presented with one default content block (or an other block configured to the page type) in the opened state.

@NightJar
Copy link
Contributor Author

NightJar commented Oct 1, 2019

Cool, this is merged now - there are two new config options to control whether a content field on any particular page appears or disappears (default).
Simple YML updates (and a corresponding flush) should see fields (re)appear where defined.

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

Successfully merging a pull request may close this issue.

8 participants