afterschoolstudio / Depot

Structured data editor built inside VS Code
Apache License 2.0
205 stars 20 forks source link

"Schema editing" and "add/remove items" disabled by default. #36

Open Vaskivo opened 3 years ago

Vaskivo commented 3 years ago

In an effort to prevent user mistakes, I think the "Allow Schema Editing" and "Add/Remove Items" toggles should be "off" by default.

At the moment, both are "on" by default.

kkukshtel commented 3 years ago

This may be something we could add into the high level config in the .vscode folder, but I'm wary of making it the default because it may make Depot feel "broken" if you have to manually select the ability to edit it when you first open it.

Are you thinking it would be a global setting or something per sheet?

Vaskivo commented 3 years ago

IMO, it would be a global setting.

And, at least, making "Allow Schema Editing" be "off" by default makes more sense. This is useful for two reasons:

  1. Preventing errors by the user.
  2. The "add column" buttons popping up when editing lists is really annoying.

I agree that having "Add/Remove Items" "off" might make depot seem broken.

kkukshtel commented 3 years ago

I'm worried that either of them off by default would make Depot feel broken. For example right now if you had schema editing off, you would be unable to add any new fields to a created sheet. I think making this global could be the right move in the settings.json folder. Where default behavior is as it is now, but you can override the individual settings in the settings.json file. Would that make sense?

Vaskivo commented 3 years ago

Well, in my mind, I see that adding, removing and editing rows happens many more times than editing the schema. But I see what you mean.

Changing the default in settings.json would be good solution for me. :)

Vaskivo commented 3 years ago

Created a PR addressing this issue.

Was this what you had in mind when we previously discussed this?

kkukshtel commented 3 years ago

This looks exactly right! I'm in the process of adding in some new fixes and changes on https://github.com/afterschoolstudio/Depot/tree/july21-update so I'll merge your PR into master once those changes are in. I had to make a few tweaks just to get everything compiling again so looking at your PR now there may need to be a few things moved around but otherwise it looks good to go.

Vaskivo commented 3 years ago

If you need any help, let me know.

kkukshtel commented 3 years ago

Feel free to tackle any of the other requested features here 😄

https://github.com/afterschoolstudio/Depot/issues