Filetreediff: expose it for users that have it enabled

[humitos]

We don't expose it to all the users yet, but we want to expose it for those that we already enabled it on the Django Admin.

[ericholscher]

So people will be able to turn this off, but not turn it back on? Seems a little weird, but fine for now I guess.

[humitos]

Yeah. The other option is to expose it to everybody 🤷🏼

[ericholscher]

Could we check for it not being null, like we talked about in the call? That way if they ever changed it from the default, they would still have a toggle.

[humitos]

Hrm, yeah, but the situation would be similar:

• we show it if it's not null
• the field renders as a 3 value (true, false, null)
• if they select null they won't be able to enable/disable it again

We would need to check if the value is not null in the backend, and in that case, render the field as a 2 value boolean. I think that could work.

[humitos]

I opened readthedocs/readthedocs.org#11988

[humitos]

Well, it seems we don't have null values in the database for some reason 🤷🏼

# community
In [1]: Project.objects.filter(addons__filetreediff_enabled__isnull=True).count()
Out[1]: 1

# commercial
In [1]: Project.objects.filter(addons__filetreediff_enabled__isnull=True).count()
Out[1]: 0

[ericholscher]

Yea.. I think the 3-selector is probably useful for these cases, but in some ways we're also just reimplementing feature flags for this use case? I think overall the main thing is knowing when a user has explicitly set it off 🤷

[humitos]

I think we should be able to use history models to know if they are using the default value or not.

Is there anything blocking us for enabling it by default to everybody? If so, we should work on that.

[ericholscher]

Yea, I'm 👍 on shipping it to everyone, once we get these polish PRs in.

[humitos]

once we get these polish PRs in

yeah, that was my question: what are the things we want to polish before shipping it? I can prioritize those tasks.

[ericholscher]

I think with ignored files and the new chunking, it's working pretty well for me. I think the main UI things that @agjohnson noted are also now fixed:

The changes in polish I'd like to see here are:

• Matching element styles to other elements. Our flyout, notifications, and this UI all look different
• Make sure edge cases like mobile display, width of file name and width filename plus chunk selection aren't too wide for the view port, etc don't make docs or the diff UI unusable

If we can confirm those are all working, I think we can probably ship it.

[humitos]

Matching element styles to other elements.

I merged the PR that matches the UI to the flyout: readthedocs/addons#526

Make sure edge cases like mobile display

I have a PR opened for this at readthedocs/addons#530

[ericholscher]

I think I'm on board with shipping this to everyone next deploy.

[agjohnson]

To clarify what I noted in our meeting, I did not want to match everything to the flyout style just yet. This effectively makes everything dark mode by default. I instead would like to have some unity on the diff UI, notifications, and search UIs. These all feel independently styled and like different experiences right now.

The flyout is special as it is always in a dark mode right now. I would like to explore a light mode flyout eventually, but it's fine if that is separate.

[humitos]

I think I'm on board with shipping this to everyone next deploy.

Do we want to enable it by default? I'm 👍🏼 on that.