-
Notifications
You must be signed in to change notification settings - Fork 2
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
Addons: allow users to opt-in into the new addons #212
Comments
This would be an easy UI to get started on. The template work will be minimal, we just need the Form and View instances. After that, templates would add a Project -> Admin -> Addons UI, basically just showing the initial Form instance. There is definitely a long term plan here and a short term plan. I talked about this in another issue that I am not finding now. In the short term our Form would be a simple form with "Enable addons beta", or maybe even "Enable flyout beta". But we wouldn't reuse this Form long term, so it wouldn't be a good candidate to build on top of. Long term, the goal would be to have a Project -> Admin -> Addons -> Flyout sub-admin page, with separate Form class for each addon configuration. I'd want to probably try with using a tabbed interface and multiple form instances first, as otherwise we need to create a lot of redundant views/templates/etc. So, I'd say it's 100% okay to start with a basic form for "Enable this beta", understanding we won't build on top of this. But, also, if we feel we already want per-addon configuration this may not be too much more work, so could be a place to start as well. |
For assignment, I'm happy to help with the ext-theme changes, but they will also be very small to start, and mostly just duplicating existing templates. @humitos would you like to start with the form/view/url additions here first? I've assigned us both for now, but it might be easy enough to add all at once too. |
I see two different approaches here:
|
I created the URL, models, view and form to prepare ourselves for the beta addons. Currently, if the `Project.addons` fields is not null we add a HTTP header to tell Cloudflare to enforce the new beta addons on this project. In the future, we can use each of the `ProjectAddonsConfig` field to let the user decide which of the addons they want to enable/disable and change their configurations. * Related: readthedocs/ext-theme#212
Show a small form to the user where they can enable/disable the new beta addons on their project. * Closes #212 * Requires readthedocs/readthedocs.org#10733
I went ahead and opened readthedocs/readthedocs.org#10733 and #213 |
This sounds like what we are wanting immediately, for this issue -- just the beta opt in/out.
And this sounds like the continuation of this work that we'll want for #211. Or we can continue thinking how we'll lay this out in code more, I don't have strong opinions on the modeling structure. Does that match what you're thinking too? |
Yeah, but I went ahead already and implemented both of them: the immediate requirement (enable/disable beta addons) and the underlying modeling needed to allow users customize the addons --which is currently not exposed to users yet. |
* Addons: allow users to opt-in into the beta addons I created the URL, models, view and form to prepare ourselves for the beta addons. Currently, if the `Project.addons` fields is not null we add a HTTP header to tell Cloudflare to enforce the new beta addons on this project. In the future, we can use each of the `ProjectAddonsConfig` field to let the user decide which of the addons they want to enable/disable and change their configurations. * Related: readthedocs/ext-theme#212 * Changes from feedback Use a `AddonsConfig.enabled` field to decide whether or not all the addons are enabled. * Docstring to document the HTTP headers to communicate with CF * Addons: update modelling and queryset * NGINX: pass the `X-RTD-Force-Addons` HTTP header * Declare the URL only when ext-theme is enabled
* Addons: allow users to opt-in into the beta addons Show a small form to the user where they can enable/disable the new beta addons on their project. * Closes #212 * Requires readthedocs/readthedocs.org#10733 * Add beta label on the menu item * Upate the notification and ask for feedback
We have deployed the new addons for projects using
build.commands
and it has been working fine. We haven't received any complain and I've already saw users jumping intobuild.commands
because they have the flyout there as well (see readthedocs/readthedocs.org#10706)Also, we've found a way to "migrate old projects/builds using the old integrations to the new one" dynamically using a Cloudflare Worker (see https://github.com/readthedocs/readthedocs-ops/pull/1402).
However, we can't do this migration automatically for all the projects because it won't look exactly the same. We will be moving the flyout from the bottom left being integrated into the navbar, to the bottom right being fixed in a floating position. I created an issue in the theme to make this integration to look exactly the same at readthedocs/sphinx_rtd_theme#1523 -- but it needs a non-trivial amount of work for that to define the exact addons API, work on JS/CSS, release a new version, wait for people to migrate to this version, etc.
We can follow a simpler path here: "allow users to opt-in into the new addons". When opt-in we can communicate these differences (look&feel changes and extra features) and they can disable if they don't like it. This will give users the features we already have in the new addons immediately to them, it will allow us to gradually roll this out and fix issues as we go and also give us feedback about that integration into the theme should be.
Related:
The text was updated successfully, but these errors were encountered: