Addons: update form to show all the options

[humitos]

Add all the configuration options to the AddonsConfigForm so the user is able to enable/disable each of the addons independently.

This is the initial version of this work. We will keep adding more configuration options to each of the addons. That's why I decided to create one section per each. In the future, instead of just a regular FieldSet we can get more fancy and use tabs instead (creating a custom class or giving it a try to https://crispy-forms-gds.readthedocs.io/en/latest/components/tabs.html)

Preview

ToDo

• get a review on the copy for each of the sections
• migrate projects with Feature flags to disable specific addons

Closes readthedocs/ext-theme#211

[agjohnson]

Noted at readthedocs/ext-theme#262 (comment)

Two things that are important here:

First, I think this interface will eventually use a tabbed interface, not fieldsets, or might even use separate form/views entirely -- we'll know more once we start adding deeper configuration options here. But for that, I am gathering fieldsets will probably be removed in the future.

Second, I wanted to discuss avoiding (and probably removing) use of Crispy layouts and the layout API. Having HTML authoring removed from template code is a bit awkward already, just given the code/template separation and repository split, but it gets much worse if we want JS/Knockout/etc code in the form.

For that, I might suggest keeping this incredibly simple for a first pass and removing fieldsets and Python HTML authoring.

[humitos]

For that, I might suggest keeping this incredibly simple for a first pass and removing fieldsets and Python HTML authoring.

How do you imagine that version? Just a bunch of inline checkboxes, as shown in readthedocs/ext-theme#262 (comment))?

[agjohnson]

Yeah, that seems like plenty 👍

It will look minimal, but should be fine -- especially with a fix for block display checkboxes. We definitely will want to improve on that later in some way in the future, and we can start mocking out what that looks like once we know what other configuration fields we want there.

[humitos]

I'm not sure how to fix the inline checkbox CSS issue, but I found where is the issue at least.

.ui.checkbox {
  display: inline-block;
}

Removing that property it displays as we want, I'd say.

[humitos]

I will move forward with the required changes on this PR and we can tackle that CSS issue in ext-theme, I guess.

[agjohnson]

Yeah, that sounds good. I agree it's probably minor and I should be able to squeeze a fix into the crispy field template before deploy

[agjohnson]

This looks good! There are a few display issues I'll follow up on tomorrow or monday.

[agjohnson]

I'd say the only thing I noticed is that this field renders as Enabled, and should probably be more descriptive regardless of the pattern we use -- Enable Addons or something.

[humitos]

Yeah, good point. I originally used the layout approach and added some text explaining this was "global". I'll check with the serializer if I can change how it renders since we are already using the help_text to explain the same thing.

[humitos]

I also thought we can use better labels for:

            "external_version_warning_enabled": _("Show a notification on builds from Pull Requests"),
            "stable_latest_version_warning_enabled": _("Show a notification on non-stable and latest versions"),

What do you think?

Also also, we can remove the enabled part from all of them and just leave the name of the addon with the checkbox?

[humitos]

[agjohnson]

I also thought we can use better labels for

Yeah those look better! Just a small note that "Pull Request" should use proper noun capitalization, "pull request builds" can be used instead. I follow what GitHub uses: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests

Also also, we can remove the enabled part

Aye, we could for this UI. Though thinking ahead to when we split it up to multiple pages/tabs, we might want the full verbose version "Flyout enabled"? I suppose I'd leave the "enabled" part on the text for now and if we want to remove it later we can. For now, the extra enabled isn't making the options any less clear.