treewide: add new css class button-row
Base on the discussion with @systemcrash @jow- on PR https://github.com/openwrt/luci/pull/7152, I have added the suggested css class button-row.
New view for the connectivity change that I originally wanted to change.
I have tested the changes for the themes luci-theme-material and luci-theme-bootstrap.
- Interface stop
- OPKG update/feeds
- Upload
When comparing the Save & Apply I noticed a problem that looks right with luci-theme-material but not so right with luci-theme-bootstrap.
luci-theme-bootstrap (NOK):
luci-theme-material (OK):
I think the problem is with the CSS! Unfortunately I have no idea how to solve this.
- [x] Each commit has a valid :black_nib:
Signed-off-by: <[email protected]>row (viagit commit --signoff) - [x] Each commit and PR title has a valid :memo:
<package name>: titlefirst line subject for packages - [x] ( Preferred ) Mention: @ the original code author for feedback
- [x] ( Preferred ) Screenshot or mp4 of changes:
- [x] Description: (describe the changes proposed in this PR)
That blue save and apply button i the OK example is virtually unreadable
That blue save and apply button i the OK example is virtually unreadable
This isn't a bug in this PR tho, it's been that for a long time in the Material theme (I run it as default) so doesn't need to be addressed in this PR in my mind.
I think the problem is with the CSS! Unfortunately I have no idea how to solve this.
It's because all list (li) elements inside the .modal class is styled as color: gray and the buttons in the Configuration / Changes dialog is wrapped as list items, which is a side-effect of trying to color something else in the modals, I haven't figured out what yet tho.
Unreadability (illegibility) bears pointing out and... if possible, fixing. I don't use that theme so I've never noticed it. 🙃
If you figure it out, pepper in some code comments to an obnoxious degree so others tweaking css benefit...
Unreadability (illegibility) bears pointing out and... if possible, fixing. I don't use that theme so I've never noticed it. 🙃
Absolutely, if it's easy to fix we should definitely do it in this PR if possible. I looked into this a while ago as part of another thing, and then I didn't find what the styling could be used for, so it may be hard to verify if there will be any regressions worse than the button illegibility by just removing it.
Just remove and break stuff then test. Should be OK ™
@systemcrash @dannil I am not a CSS expert. So I'm not able to fix this without delving deeper into CSS, which I want to avoid. Any help to fix this in CSS will be appreciated
@feckert So your wording is still problematic (in English), although we could get away with:
Apply, reverting if connectivity is lost
Apply, reverting in case of connectivity loss
As you might guess, I want to avoid after here because after makes it seem like an implied consequence of this action, i.e. like it (connectivity loss) should happen.
Apply, reverting after connectivity loss
Apply, but revert after connectivity loss
@systemcrash thanks for the suggested wording. I have update the pullrequest.
Np.
Did one of the buttons function change?
@systemcrash Thank you for seeing this. I made a mistake when rebasing. It should be OK now.
Ok. Better. I think that the last button would probably do well called "Apply unchecked", as a suggestion - this wording has enough contrast with the middle button to make its purpose distinct.
@systemcrash done
Looks good to me. Merge at your leisure.
This change will cause luci-proto-wireguard to break.
Will have look. Thanks for feedback
There seems to be a regression in the state change on the "Upload" button in "Flash new firmware image", it doesn't toggle to the "active state" when you upload a valid sysupgrade. I haven't investigated if it's a general issue. It's still possible to click it tho to proceed so it's a styling thing.
Before:
After:
This change will cause luci-proto-wireguard to break.
@feckert I went back and double-checked the latest wireguard changes, and buttons there still work fine, so it's related to changes from this PR. Some of the buttons in wg might lack some essential styling, perhaps?
I have managed to display the StackedSubModul of the wireguard peers again by adjusting the querySelector (https://github.com/openwrt/luci/pull/7200). But now there is another problem with the Back button of StackedSubModul hanlding of the wireguard peer page. It could be closed but the previous SubModul close button does not! I have to reload the whole page.
The error must be somewhere here! https://github.com/openwrt/luci/blob/master/modules/luci-base/htdocs/luci-static/resources/form.js#L3204-L3217
I broke it, but @jow- can you maybe help here?
Maybe revert for now, and queue up the cumulative relevant changes into the new PR 7200
I have update the PR #7200, that should fix the regression for sub modal handling that is used in the wireguard proto. @zdz2019 please try