Don't add default content to grids #4177

kjac · 2019-01-22T06:40:47Z

Prerequisites

I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes: #4154

Description

As described in #4154 the grid misbehaves when the grid configuration has only one grid layout and one row configuration - like this:

Given that configuration, the grid property editor adds a default value to the grid - like this:

With this PR applied, the same operation looks like this:

Testing this PR

Create a page with grid configured as shown above, and verify that the default value is not added to the grid.
Publish the page.
Given the template below, verify that Model.Content.HasValue("[gridPropertyAlias]") returns false when the page is rendered.

@inherits Umbraco.Web.Mvc.UmbracoTemplatePage
@{
	Layout = null;
}
<html>
    <body>
        <h1>@Model.Content.Name</h1>
        Has value? @Model.Content.HasValue("gridContent")
    </body>
</html>

emmaburstow · 2019-01-22T10:45:44Z

Yo @kjac

Sorry, running out of greetings for you. I might google some for the next PR. We'll test and all that and then merge if it's all gravy.

Em

kjac · 2019-01-22T10:55:14Z

Hah! You do that 😋

nul800sebastiaan · 2019-01-30T17:57:13Z

Had a look at the old commits and it looks like the intention was: "you only have 1 choice anyway, so let's get you started immediately".

We'll discuss in out regular meetings at HQ and get back to you!

kjac · 2019-01-30T20:17:52Z

@nul800sebastiaan sounds very much in line with the code I went through working on this PR.

It's a valid argument. But I think the HasValue() counter argument is a better one 😄

nul800sebastiaan · 2019-01-30T20:51:21Z

Haha yeah, but the question is: why should we store anything if there's nothing actually added to the grid.

Also, the question is: do we want to keep helping people with one less click when they create a new page with a grid with only 1 option. Probably yes. And if we can do that without storing values as long as they don't add anything to that grid then that's probably the way to go.

kjac · 2019-01-30T20:57:08Z

Sounds legit. I'll think of something... surely it should be possible to give everyone the best of both worlds.

kjac · 2019-02-01T14:44:34Z

@nul800sebastiaan so I've been digging a bit more into this. And I can't see how we can support both "add default content" and "don't save default content" or "don't let default content count towards HasValue".

The grid will automatically add the first row if you have only one row config. If that row config allows only one editor, this editor will also automatically be chosen. And therein lies the issue: It's going to be a pain to figure out if the default row + editor combo constitutes an "empty" value, considering that any given editor not necessarily needs an input value to render something meaningful in the frontend.

On another note: When I add another row, shouldn't the grid automagically pick the one and only row applicable config?

nul800sebastiaan

@kjac We had a chat about this at HQ this week and the behavior is almost correct:

If there's only ONE possible layout, then automatically choose that layout
Then after that it is fine to give the "Add row" choice instead of adding the only possible row

That makes sure we'll never add an empty editor that is never used and this whole problem is solved. Can you update please? 😁

kjac · 2019-03-22T11:23:54Z

@nul800sebastiaan I tried to explain this in my last comment:

We can add the default layout if there is only one layout available and still handle Can't save empty grid layout #4154 simply by stripping out the empty layout if present on save.
We can't add the default editor in case there is only one editor allowed in the layout and still handle Can't save empty grid layout #4154, because it's impossible to know if the editor is empty or not.

This leaves us with the following options:

Don't add the default layout (this PR as-is).
Add the default layout but not the default editor (this PR can easily be tweaked).
Add the default layout and the default editor and leave Can't save empty grid layout #4154 unsolved (simply discard this PR)

Choices, choices... which to choose? I think 2 is the worst option of the three 🤔

nul800sebastiaan · 2019-03-22T11:31:16Z

@kjac #2 is exactly what we would love! 👍

The idea is: you have only 1 layout choice and after looking at people's implementations for a few years: that's usually the case. So then you don't need to show it as a choice. Most of the times the next step would be adding a row and most people will have 2 or 3 row choices, so it would always be something they'd need to choose anyway. If there's only 1 row choice then it's not so bad to make it a deliberate choice to add something. And it solves the issue of adding something "empty" that will look weird afterwards.

kjac · 2019-03-22T12:40:15Z

@nul800sebastiaan so... after even more digging into this, it turns out we can't exactly do option 2 very elegantly.

Long story short: When we have an "empty" (default) grid, we have to pass a null value to the server in $scope.model.value, or make the grid editor value converter explicitly convert the default value to null, to make Content.HasValue("...") return false. No matter if it's done client or server side, we'll end up with this kind of flickering, because the view model is temporarily set to null:

nul800sebastiaan · 2019-06-06T10:30:30Z

Hey there @kjac !

I'm writing today after announcing at Codegarden, our yearly conference, that the upcoming version 7.15.0 will be the last Umbraco v7 release with new features in it. From now on all our efforts will be focused on version 8 instead. 🎱

We're wrapping up the 7.15 version at the moment and unfortunately these proposed changes won't be able to make it in there.

We'd like to extend a big thank you the efforts on this so far and we would love it if you could have a look at porting this feature over to v8 instead. Of course if you have any questions on where to start then feel free to ask on the forum, we monitor all questions there and we're happy to assist.

Thanks again for the work in this PR and we hope to see you contributing to v8 some time soon!

Best, Sebastiaan and the Umbraco CMS team.

Don't add default content to grids

6c8e094

kjac mentioned this pull request Jan 22, 2019

Can't save empty grid layout #4154

Closed

nul800sebastiaan added the state/hq-discussion-ux label Jan 30, 2019

nul800sebastiaan requested changes Mar 20, 2019

View reviewed changes

nul800sebastiaan removed the state/hq-discussion-ux label Mar 22, 2019

zpqrtbnk changed the base branch from dev-v7 to v7/dev March 31, 2019 16:33

nul800sebastiaan added the type/feature label May 31, 2019

nul800sebastiaan closed this Jun 6, 2019

kjac mentioned this pull request Nov 18, 2019

Grid layouts and fallback language #7158

Closed

umbrabot added the type/feature label Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't add default content to grids #4177

Don't add default content to grids #4177

kjac commented Jan 22, 2019

emmaburstow commented Jan 22, 2019

kjac commented Jan 22, 2019

nul800sebastiaan commented Jan 30, 2019

kjac commented Jan 30, 2019

nul800sebastiaan commented Jan 30, 2019

kjac commented Jan 30, 2019

kjac commented Feb 1, 2019

nul800sebastiaan left a comment

kjac commented Mar 22, 2019

nul800sebastiaan commented Mar 22, 2019

kjac commented Mar 22, 2019

nul800sebastiaan commented Jun 6, 2019

Don't add default content to grids #4177

Don't add default content to grids #4177

Conversation

kjac commented Jan 22, 2019

Prerequisites

Description

Testing this PR

emmaburstow commented Jan 22, 2019

kjac commented Jan 22, 2019

nul800sebastiaan commented Jan 30, 2019

kjac commented Jan 30, 2019

nul800sebastiaan commented Jan 30, 2019

kjac commented Jan 30, 2019

kjac commented Feb 1, 2019

nul800sebastiaan left a comment

Choose a reason for hiding this comment

kjac commented Mar 22, 2019

nul800sebastiaan commented Mar 22, 2019

kjac commented Mar 22, 2019

nul800sebastiaan commented Jun 6, 2019