Hi,

please note: I added my thoughts below.

On 22.03.2021 12:36, Michael Tremer wrote:
> Hello,
> 
>> On 21 Mar 2021, at 18:25, Matthias Fischer <matthias.fischer(a)ipfire.org> wrote:
>> 
>> Based on https://bugzilla.ipfire.org/show_bug.cgi?id=12588
>> 
>> Hi,
>> 
>> in the meantime I worked on this - a bit.
> 
> Very good idea. This page is quite essential, but the design is quite complicated and difficult to use.
> 
>> I've made several changes and adjustments which could be implemented one
>> by one - if they are wanted/accepted:
>> 
>> - Changed icons to buttons => better readability.
> 
> Yes. I am not sure whether “Backup” is a good choice, because it could be interpreted as a noun. Maybe “Backup Now” or “Update Backup” would be a better choice. Does anybody have any ideas?

"Save config"? Important: when using buttons, the text should not be too
long. Translation strings for normal-backup/restore and
addon-backup/restore *must* differ.

>> - For buttons to work, I added two lang strings for button labeling.
> 
> No problem here.
> 
>> - Cleaned up the code a bit, some if-queries were unnessecary (if
>> ($cgiparams{'ACTION'} eq...).
>> 
>> - Changed the order of the (Addon-)buttons to "Download - Save - Delete"
>> (old: Download was in the middle).
> 
> What is the rationale here?
> 
> Why is this order better?

I changed this because nearly every page on my IPFire has the 'delete'
function - mostly icons - on the far *right* side (e.g.: wio, DNS, dhcp,
hosts, wake on lan, etc.) So I thought that placing the 'delete' button
in the *middle* made no sense here. Too often, I clicked the wrong icon
on the backup page. With text buttons and the changed placement that
didn't happen anymore.

>> - Marked IPF-files bold => better readability (I already sent this in
>> one day: => https://patchwork.ipfire.org/patch/3124/).
> 
> Can we not have a proper table here? Like this:
> 
> | Package    | Last Backup      | Size |                              |
> | wio        | 2021-03-22 11:31 | 8 kB | [Download] [Backup] [Delete] |

I won't have much time in the future, but I'll "see what I can get". ;-)

>> - Added query for deleting a backup file (onclick=\"return confirm...).
> 
> Very good idea, too.

Not from me - just using it. That's what friends are for... ;-)

> Can we make this a function that we can add to various other places, too? Like firewall rules?

Yep. Working here in the following GUIs: aliases, backup, connscheduler,
ddns, dhcp, firewall, hosts, media, proxy, qos, routing, urlfilter,
wakeonlan, wio and wireless.

Best,
Matthias

>> Details in the attached screenshot.
>> 
>> Proposed ToDo - as a start: Alter the info text for restoring to make
>> clear that only the addon *configs* are restored, not the addons themselves.
> 
> Yes.
> 
>> Thoughts? Opinions?
> 
> Very good ideas. Please keep them all in individual patches so that they can be reviewed one by one :)
> 
> -Michael
> 
>> 
>> Best,
>> Matthias
>> <backup-cgi-tuned.png>
>