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

Cleanup the default datatypes (remove the ones that need config input, add ones that can be used across implementations) #8729

Closed
TimGeyssens opened this issue Aug 26, 2020 · 22 comments

Comments

@TimGeyssens
Copy link
Contributor

Hey,

When you have a fresh Umbraco install, some datatypes are already in there. Some of these don't make sense to have (like a dropdown without options).

Others seem to be missing that could make sense:
single media picker
single image only media picker
single url picker

To get some more context on the request check this twitter thread: https://twitter.com/timgeyssens/status/1298231904827777024

@bjarnef
Copy link
Contributor

bjarnef commented Aug 26, 2020

Usually I create instances as the following:

  • Image Picker
  • Multi Image Picker
  • Single Url Picker
  • Multi Url Picker
  • Email Address (not sure if this instance exists by default now)

@madden-tom
Copy link
Contributor

I always add a checkbox and name it Checkbox so there is then one present that can be re-used. This used to be present in v7 and this reduces the possibility of inadvertently ending up with a long doctype specific name for a simple checkbox

@TimGeyssens
Copy link
Contributor Author

yup good one @madden-tom

@nielslyngsoe nielslyngsoe added the category/ux User experience label Aug 26, 2020
@TimGeyssens
Copy link
Contributor Author

Main point would be, this can be confusing for new folks... and also a waiste of time for the oldies... (setting up the same stuff each time)

@TimGeyssens
Copy link
Contributor Author

Also seems an easy thing to change, from what I can see it is handled here: https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs#L249 (inserting the default data in the database)

@nul800sebastiaan nul800sebastiaan added state/needs-investigation This requires input from HQ or community to proceed type/feature labels Aug 27, 2020
@nul800sebastiaan
Copy link
Member

We'll have to have a chat at HQ and see what we think makes sense.

I'll link this to #6658 since we never really seem to have concluded anything there either.

@TimGeyssens
Copy link
Contributor Author

@nul800sebastiaan ok sweet! The ones that are just empty don't make sense imho, since they just end up confusing folks...adding the ones that avoid people creating the same basic ones over and over again...

@TimGeyssens
Copy link
Contributor Author

@nul800sebastiaan also why move this to an internal invisible channel... keep it OPEN and FRIENDLY :)

@nul800sebastiaan
Copy link
Member

We'll transparently, openly and in a friendly way let you know what the outcome of that chat is.

@TimGeyssens
Copy link
Contributor Author

Sounds good just keep it RESPECTFUL

@nul800sebastiaan
Copy link
Member

And just to set some expectations, our next meeting around things like this is in a week and a half and there's community representation from the core collaborators team in this meeting.

@nul800sebastiaan
Copy link
Member

Sounds good just keep it RESPECTFUL

I am not sure why you are quoting our values at me? I would hope that it's understood that we alway try to live up to the values we've set for ourselves.

@TimGeyssens
Copy link
Contributor Author

both sounds good! looking forward to the outcome!

@nul800sebastiaan
Copy link
Member

Alright, we've had a good chat about it at HQ and talked about the history of datatypes as well, fun fact: the only reason we have "hide label" on all datatypes right now is because in version 2 (yes, two!) of Umbraco, the RTE needed to be on it's own tab and it didn't make sense to show the label since the tab name was already the label. So we're still dragging along that wonderful bit of legacy right now.

Anyway, back to the matter at hand: yes, it makes total sense to only pre-install datatypes that don't need a configuration. This will make it faster to add those datatypes to the doctype not having to show an extra step of configuring it.

These datatypes would be:

  • Email address
  • File upload
  • Label
  • Member group picker
  • Member picker
  • Rich text editor (as it is today, or do we think it needs to not be there/needs updating?)
  • Textarea (as it is today)
  • Textbox (as it is today)

Now, we're looking for some input:

  • Date/time - should we make a "Date + time", "Date", "Time" separately, and would we always enable "offset time"? It seems like there's too many options people would want to tweak, especially with the date format
  • Markdown editor - it has a bit of config but would it make sense to set it up as an editor with a preview, or do we want people to always choose a config?
  • Media picker, I feel like Bjarne has it right: Image Picker and Multi Image Picker, but a lot of people also want the start node not to be the root of the media section. Then again, that would mean there's something generic available and they can tweak it afterwards, seems like a fair trade-off, right?
  • Url Pickers - again, I think Bjarne got it right, Single Url Picker, Multi Url Picker and by default the two toggles are off I think, agreed?
  • Tags? Could be good to add with the default group and stored as json, what do you think?
  • Toggle, as Tom mentions.. is it True/Fale? Is it Yes/No? What should the label be? Or no label? And the name, checkbox.. it's a toggle now.. I know checkbox is in muscle memory when you're looking for it though.. not sure

Did I forget anything?

Testing
There's one note that we need to consider / test after it's done: we need to make sure that the starter kit still works with a fresh install, we might be missing some datatypes if they're not bundled in the package. I doubt it, but it will be good to check.

@nul800sebastiaan nul800sebastiaan added community/up-for-grabs and removed state/needs-investigation This requires input from HQ or community to proceed labels Sep 10, 2020
@umbrabot
Copy link

Hi @TimGeyssens,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

@TimGeyssens
Copy link
Contributor Author

Nice, thanks for the update @nul800sebastiaan

"Date + time", "Date", "Time" , yes, not sure about the offset time setting... what do other folks think @bjarnef @madden-tom ?
Tags, yes makes sense
Checkbox default true, checkbbox default false?

Good point about checking the starter kit, do we also need to consider UNO?

@leekelleher
Copy link
Member

FYI, these are the data types that are used by default Media/Member types...

private static IEnumerable<int> GetNonDeletableSystemDataTypeIds()
{
var systemIds = new[]
{
Constants.DataTypes.Boolean, // Used by the Member Type: "Member"
Constants.DataTypes.Textarea, // Used by the Member Type: "Member"
Constants.DataTypes.LabelBigint, // Used by the Media Type: "Image"; Used by the Media Type: "File"
Constants.DataTypes.LabelDateTime, // Used by the Member Type: "Member"
Constants.DataTypes.LabelDecimal, // Used by the Member Type: "Member"
Constants.DataTypes.LabelInt, // Used by the Media Type: "Image"; Used by the Member Type: "Member"
Constants.DataTypes.LabelString, // Used by the Media Type: "Image"; Used by the Media Type: "File"
Constants.DataTypes.ImageCropper, // Used by the Media Type: "Image"
Constants.DataTypes.Upload, // Used by the Media Type: "File"
};
return systemIds.Concat(GetNonDeletableSystemListViewDataTypeIds());

@bjarnef
Copy link
Contributor

bjarnef commented Sep 10, 2020

Regarding Media Picker it would be great to keep it simple and not worry about start node (we need some content for this, which we don't have in a clean install).
Most often we just want to select a single or multiple images. Other configurations are probably more specific to each project, but not something all needs. Maybe sometime in a future version the configuration is simpler, yet still flexible: #8471

A simple "Date and time" and "Date" would also be useful. Not sure if we want "offset time" to be enabled/disabled by default.

Maybe we would also have a Multiple Member Picker #7634 but in that case in with be fine to only have a single Member Picker by default.

Some of the datatype instances are used in core. In past bad things would happen if you accidently deleted these - haven't checked recently if all core datatypes are locked 🙈

@TimGeyssens
Copy link
Contributor Author

@leekelleher thanks :)

@bjarnef guess locking would be a different issue? If it is possible to delete?

@NikRimington
Copy link
Contributor

I've had a quick skim over this and one thing I noticed is that Image Cropper was a default data type wasn't listed (I might have missed it and if so sorry), but that should still be created as the Image Media type needs it. IMO

@TimGeyssens
Copy link
Contributor Author

Yes we need that one :) , good call!

@umbrabot
Copy link

Closing this in relation to #7634 - make sure to read the close reason on #7634

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

No branches or pull requests

9 participants