octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.03k stars 2.21k forks source link

Plugin Freeze/Unfreeze #2714

Closed pjrola closed 6 years ago

pjrola commented 7 years ago
Expected behavior

Ability to "unfreeze" a plugin being currently frozen from updates.

Actual behavior

Update hop up displays currently frozen plugin with no clear way to unfreeze. A checkbox displays however it is un checked. This checkbox should be checked. When unchecking it should update the system_plugin_versions table. Currently you have to manually navigate into the table to change the value from 1 to 0 in the is_frozen column.

Reproduce steps

Navigate to Settings > Updates & Plugins click manage plugins, choose a plugin and click Enable-Disable. Notice freeze updates checkbox appears. Click Freeze updates note there doesn't seem to be a way to un freeze after a plugin has previously been frozen.

October build

1.0.1

daftspunk commented 7 years ago

I can see how this might be confusing UX, but when you leave the checkbox unticked and click Apply, it will unfreeze them. Perhaps we can include a note that makes this more obvious.

LukeTowers commented 7 years ago

@daftspunk Could we check the checkbox in that dialog when it has been frozen so that it is more obvious? Currently it's confusing (even to me) when I'm using that dialog and I'm uncertain if my changes have taken effect, or how to reverse them.

daftspunk commented 7 years ago

If you have selected 1 frozen and 1 unfrozen, is the checkbox checked or unchecked?

LukeTowers commented 7 years ago

Right now, it's unchecked regardless of how many plugins are selected. I think we'd have to just implement the current status indicator if one plugin is selected and improve the user interface (maybe a better explanation of what will happen) for when more than one plugin is selected.

daftspunk commented 7 years ago

we'd have to just implement the current status indicator if one plugin is selected

I'm on board with this!

Denoder commented 6 years ago

@daftspunk would this be a good way to implement it?

each plugin can be toggled rather than doing a select/deselect all. Would a swtich be good or do you wan to use a checkbox

LukeTowers commented 6 years ago

@Teranode a switch is better, but I think it could probably be made even simpler. Would you be able to do it in a table layout?

Plugin Freeze Updates Disable Plugin
RainLab.User Off Off
Uxms.Restful Off On
Denoder commented 6 years ago

@LukeTowers

would this work, also changed the words to a yes/no since its more a question where you want to do an action

Denoder commented 6 years ago

Final Product(?)

LukeTowers commented 6 years ago

That looks better, but now to take it a step further a couple of things: Plugins should be left aligned, everything else is so it makes more sense for it to be as well. What do you think about the configuration happening on every change of the switch via AJAX request? On that note, how about integrating those as a Switch column type displaying it directly in the plugins list itself as an idea? Then perhaps having the popup only be for bulk operations? Thoughts?

oliverpool commented 6 years ago

(novice view) I would suggest to inverse all the switches: Update to new versions and Plugin enabled. That way the pop-up will look greener.

Otherwise the first feeling is "so much red? something must be wrong!"

TimFoerster commented 6 years ago

I would prefer

Plugin Updates enabled  Plugin enabled
RainLab.User on / yes (green) on / yes (green)
Denoder commented 6 years ago

(novice view) I would suggest to inverse all the switches: Update to new versions and Plugin enabled. That way the pop-up will look greener.

Otherwise the first feeling is "so much red? something must be wrong!"

This is a choice left completely up to you, those texts can be changed as they are in the lang folder. Im also not too keen on wanting to change core css for something that minor. @TimFoerster @oliverpool

That looks better, but now to take it a step further a couple of things: Plugins should be left aligned, everything else is so it makes more sense for it to be as well. What do you think about the configuration happening on every change of the switch via AJAX request? On that note, how about integrating those as a Switch column type displaying it directly in the plugins list itself as an idea? Then perhaps having the popup only be for bulk operations? Thoughts?

I could do it on every ajax request. Someone suggested it be centered aligned :/ . and ill see about doing the switch column type, but for now lets focus on the popup that can take care of bulk/singluar operations.

so im left with a few choices on the matter and im not good with deciding people's personal preference so ill include my own in the screenshots:

1.

2.

3.

4.

4 options and only one to choose, I personally like #4. What are your thoughts @LukeTowers @TimFoerster @oliverpool

oliverpool commented 6 years ago

I also prefer #4.

This is a choice left completely up to you, those texts can be changed as they are in the lang folder.

You can change the text, but you can't inverse the meaning of the switch: I think that an activated, updatable plugin should look green (which is the Yes value)

Denoder commented 6 years ago

I don't know, I wanna have @LukeTowers input on this. I just want to focus on function. That visual aspect seems too minor for me to change it. I mean i get what you are saying

LukeTowers commented 6 years ago

@Teranode I like #4 the best too. Could you vertical align middle the plugin names?

Let's take a step back here for a moment though, could we perhaps take some design inspiration from WordPress (gasp I know, the sacrilege 😆 )? I know you'd like an improvement on the existing UX, and really anything would be; but if we're going through the trouble of making the changes we may as well try to make it as good as possible.

Managing the status of plugins on the listing page itself works well and makes a lot of sense, and then bulk functionality can be handled through an action that's triggered on the selected plugins from the listing page. Thoughts on that?

I appreciate your contributions on this!

Denoder commented 6 years ago

@LukeTowers

Also, I suggest going into the storm css and changing

.table > thead > tr > th, .table > tbody > tr > th, .table > tfoot > tr > th, .table > thead > tr > td, .table > tbody > tr > td, .table > tfoot > tr > td

vertical-align to middle, because the list and the regular table is affected by this. Right now its on top.

LukeTowers commented 6 years ago

@Teranode the changes would be in https://github.com/octobercms/october/blob/develop/modules/system/assets/ui/less/list.base.less#L29 for the vertical align, but I don't know if we want to change that 100% by default. Could you make the change, recompile the assets, and then test across lists in the backend to see if anything looks out of position with the change?

As for the toolbar buttons, I was envisioning a dropdown "Select action..." that has you pick from "Enable plugins | Disable Plugins | Freeze Updates | Unfreeze Updates" (keeping the remove action as it's own separate dangerous button) and then popping up a confirmation dialog saying "Are you sure you want to $action the selected plugins?"

Thoughts on the above? Great work so far, I really appreciate the work you're putting into this!

Denoder commented 6 years ago

@LukeTowers well i borrowed your 'bulk-actions' js from the user plugins file. an issue i have with that is when i use one of the bulk action the separate 'remove' button stays as if an item is checked, so i had to move it to the dropdown (with a confirmation so users know that they're deleting it)

For the css there's no probelms unless someone starts adding a margin on an element in the td but thats for any of the vertical align options, but Im still not keen on touching the css/js files so i just made the change inline.

LukeTowers commented 6 years ago

@Teranode can't you unselect the affected plugins after performing a bulk action to fix that issue? I would like it in a different button altogether given the gravity of accidentally pressing it. If it's in the same dropdown as the other actions with the same confirmation dialog, then it's not marked important enough from the other actions. Any thoughts on that? Also, do you have a screenshot with the dropdown UX?

Denoder commented 6 years ago

@LukeTowers

after the update is done via ajax the checkbox is unselected (its a refresh in that container), but the remove button acts as if the checkboxes are checked because it wasnt in that update

Alright so that should do it, the toolbar will refresh when an action is performed. (Changed screenshot)

LukeTowers commented 6 years ago

@Teranode Looking pretty awesome! Question, you seem to have added a new JS control bulk-actions, any reason why you didn't use the dropdown control instead? http://octobercms.com/docs/ui/dropdown.

Denoder commented 6 years ago

@LukeTowers it was kind of a last mineute thing, tryna do this before i get bombarded with work, then i found out i still got 2 days, so ill take a look at that dropdown later, unless youre okay with the bulk

petehalverson commented 6 years ago

Well done @Teranode! I really like this new functionality. I hadn't come across it until I started toying with the develop branch recently...

We frequently freeze plugins on client projects to avoid possible compatibility issues that may arise and go untested if an administrator updates them blindly.

Curious. Would anyone else see the value of introducing a config setting similar to disableCoreUpdates to freeze all plugins by default?

LukeTowers commented 4 years ago

@petehalverson I would see value in a global setting to disable plugin updates through the backend.