luci-app-opkg: add warning about modifying packages

[dannil]

A proposal for a fix for #6413

It is a well-known fact that opkg doesn't check ABI-compatibility between dependencies and upgrading packages in a stable release is therefore not recommended. In addition, installing and removing packages blindly may result in rendering the device inaccessible as well, so caution should be taken when doing this.

This commit adds a warning which should help notify especially new users about these behaviors with a link to the wiki which advises to not upgrade packages.

See https://openwrt.org/meta/infobox/upgrade_packages_warning for more info, which is the same link that is used in the second paragraph.

Tested on Chrome and Firefox, both desktop and mobile (Android). All CSS used is theme-compatible and should therefore look at least decent across different themes.

First iteration of design, not relevant anymore

BootstrapLight:

BootstrapDark:

Material:

OpenWrt2020:

[yurtpage]

Luci has UI notification API that can be used like:

 ui.addNotification(null, E('p', 'Werning!'), 'error');

Maybe you can use it instead?

[systemcrash]

Not optimal if it causes a pop-up or a button that must be clicked away.

[dannil]

Luci has UI notification API that can be used like:

 ui.addNotification(null, E('p', 'Werning!'), 'error');

Maybe you can use it instead?

Having a pop-up show up every time you navigate to the updates tab will most likely be very annoying to the user.

[yurtpage]

I agree and beside this IMHO it still makes sense: users not so often opening the page and at least they may close the notification. It's may be sorted later to remember that a user dismissed a notification.
This will also make a code shorter and easier to understand and maintain.
Just my five cents, the PR it good as for me

[jow-]

Please also add warning banners that installing packages and removing packages can cause soft bricks or break access to the ui.

[dannil]

Please also add warning banners that installing packages and removing packages can cause soft bricks or break access to the ui.

Great suggestion, I'll look into adding that.

[systemcrash]

Shall we add more banners and warnings that using openwrt will also potentially soft brick their devices? 😉 Please also add warning banners that *installing* packages and *removing*

…

packages can cause soft bricks or break access to the ui.

[dannil]

I've updated the PR title, commit message, and screenshots to reflect the suggestion by @jow-

[dannil]

@stokito @systemcrash @jow- is there any other changes we want or does it make sense to start thinking of merging it?

[stokito]

Replace <a %s"> with <a %s>

[systemcrash]

Is it possible to reduce the warning text to a concise nugget? That page is now exploding with lines, colours and text and this does not make it easier to handle. How about:

Warning! Package operations can break your system.')),
				E('p', _('Read <a %s>this</a>.').format(

[dannil]

Is it possible to reduce the warning text to a concise nugget? That page is now exploding with lines, colours and text and this does not make it easier to handle. How about:

Warning! Package operations can break your system.')),
				E('p', _('Read <a %s>this</a>.').format(

Absolutely, I like that idea.

[systemcrash]

And now... to acknowledge this big warning dialogue.

@yurtpage suggested

Luci has UI notification API that can be used like:

 ui.addNotification(null, E('p', 'Werning!'), 'error');

Maybe you can use it instead?

But maybe something more resilient. Like an 'acknowledge' button, that when clicked, stores a boolean flag in the config that determines whether the warning is shown at page load. We've done our diligence, the user can hide the warning.

What do you think @dannil ?

[dannil]

And now... to acknowledge this big warning dialogue.

@yurtpage suggested

Luci has UI notification API that can be used like:

 ui.addNotification(null, E('p', 'Werning!'), 'error');

Maybe you can use it instead?

But maybe something more resilient. Like an 'acknowledge' button, that when clicked, stores a boolean flag in the config that determines whether the warning is shown at page load. We've done our diligence, the user can hide the warning.

What do you think @dannil ?

Yes, I've actually thought about that too. I think we could do something akin to how the "Confirm" button behaves in warning dialogs (such as when you don't have a password set on your root user), here's an example from nftables:

Do we think it would be necessary to have a checkbox somewhere else (like under the "Configure opkg..." dialog) that would enable users to show the warning again? I personally think it shouldn't be needed.

[stangri]

1. I believe a nicer way would be: Warning: package operations can break your system (more information). and link the wiki article from more information?
2. If not, is it possible to move the Read this text to the line before and avoid the line break?

[systemcrash]

Do we think it would be necessary to have a checkbox somewhere else (like under the "Configure opkg..." dialog) that would enable users to show the warning again? I personally think it shouldn't be needed.

Unnecessary to show again. A factory reset or someone who knows what they're doing is sufficient. Your proposal is also good. 👌

1. I believe a nicer way would be: Warning: package operations can break your system (more information). and link the wiki article from more information?
2. If not, is it possible to move the Read this text to the line before and avoid the line break?

Single line looks better 🥳

[stokito]

Honestly I think it would be better to make same introduction paragraph as other pages have:

<div class="cbi-map-descr">
Install additional software and upgrade existing packages with opkg.. 
<br><b>Warning!</b> Package operations <a href="href="https://openwrt.org/meta/infobox/upgrade_packages_warning" target="_blank" rel="noreferrer">can break your system</a>
</div>

I.e. the warning can be just a sentence in the introduction.

We may add more important notes e.g. about feeds and disk space usage.
Given that this UI messages are more likely to be translated than a Wiki page it will improve cautious of users. That's why we need to add at least a few words that ABI may broken and that core modules shouldn't be updated at all. Yes, wiki explains this better but who reads it?

BTW the article in Wiki is in inbox e.g. in "draft" state. On the wiki itself not so much of information about package management https://openwrt.org/docs/guide-user/additional-software/start

[systemcrash]

Honestly I think it would be better to make same introduction paragraph as other pages have:

Also good. Less intrusive yet ever present.

[dannil]

Honestly I think it would be better to make same introduction paragraph as other pages have:

<div class="cbi-map-descr">
Install additional software and upgrade existing packages with opkg.. 
<br><b>Warning!</b> Package operations <a href="href="https://openwrt.org/meta/infobox/upgrade_packages_warning" target="_blank" rel="noreferrer">can break your system</a>
</div>

I.e. the warning can be just a sentence in the introduction.

We may add more important notes e.g. about feeds and disk space usage. Given that this UI messages are more likely to be translated than a Wiki page it will improve cautious of users. That's why we need to add at least a few words that ABI may broken and that core modules shouldn't be updated at all. Yes, wiki explains this better but who reads it?

BTW the article in Wiki is in inbox e.g. in "draft" state. On the wiki itself not so much of information about package management https://openwrt.org/docs/guide-user/additional-software/start

I like this as well, especially the less intrusive part. Also, I've had trouble to implement a UCI config for opkg (since the package itself doesn't define this I need to do some hacky solution with creating the file from the LuCI app, which I don't like) which can entirely be avoided with the above suggestion.

I'll implement this solution in the following days since it's both much less intrusive and way easier technically to implement since we don't need the whole "Hide" logic.

[systemcrash]

Merged. Thanks @dannil !