Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BasicUI] Move most of app settings to per-device (browser) settings #2243

Merged
merged 1 commit into from
Mar 3, 2024
Merged

[BasicUI] Move most of app settings to per-device (browser) settings #2243

merged 1 commit into from
Mar 3, 2024

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Dec 29, 2023 •
edited
Loading

Closes #1832
Closes #1930
Closes #2342

A new page is available to show and update all per-device settings. This page is accessible through the sitemap list page or the home page of any sitemap.
Basic UI theme now matches the dark mode of Main UI. It can be updated either in Main UI or in the new page of Baisc UI.
Web Audio can also be enabled either in Main UI or Basic UI.

The iconify setting is now enabled by default. The script to handle iconify icons is now loaded in sitemap pages only if the iconify setting is enabled.

New settings are also available to control the size of the chart and the size of content (text, line) in the chart.

A new setting is also available to enable a bigger font size.

Signed-off-by: Laurent Garnier [email protected]

@lolodomo lolodomo added enhancement New feature or request basic ui Basic UI labels Dec 29, 2023
@lolodomo lolodomo requested a review from a team as a code owner December 29, 2023 14:00
@lolodomo lolodomo marked this pull request as draft December 29, 2023 14:00
@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch 4 times, most recently from ee91e3d to afe6eee Compare December 31, 2023 12:03
@lolodomo lolodomo changed the title [BasicUI] Move most of app settings to per device (browser) settings [BasicUI] Move some app settings to per-device (browser) settings Dec 31, 2023
@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch 2 times, most recently from 31795e0 to bb3d582 Compare January 6, 2024 13:01
@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch 13 times, most recently from 390bfb3 to 9736508 Compare February 5, 2024 19:21
@lolodomo lolodomo changed the title [BasicUI] Move some app settings to per-device (browser) settings [BasicUI] Move most of app settings to per-device (browser) settings Feb 5, 2024
@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch 5 times, most recently from ca80718 to c4778a7 Compare February 7, 2024 21:02
@lolodomo lolodomo marked this pull request as ready for review February 7, 2024 21:06
@lolodomo
Copy link
Contributor Author

lolodomo commented Feb 7, 2024 •
edited
Loading

This PR is finally ready for review.

The home icon at top left has been replaced by a setting icon.

image

When clicking on it, you open the page with all settings:

image

image

In case you open the settings page in a separate tab or window, your changes are immediately applied to your other tab/window opened on Basic UI.

@lolodomo lolodomo requested a review from kaikreuzer February 7, 2024 21:22
@lolodomo
Copy link
Contributor Author

lolodomo commented Feb 7, 2024 •
edited
Loading

I have of course tested all these settings.

Regarding the "web audio" setting, first you have to know that the web audio is ready only when you are on a sitemap page, not when you are on the settings page or the list of sitemaps page. Nothing new here. The enabling/disabling of the "web audio" is working well in Chrome and Edge. In firefox on Windows 10, you have in fact to restart Firefox after enabling/disabling it to have it working, maybe a bug in Firefox.

If you are in mode auto for the theme, Baisc UI will now automatically change theme without the need to refresh the page.

@lolodomo
Copy link
Contributor Author

lolodomo commented Feb 7, 2024 •
edited
Loading

Only 2 settings remain on server side:

  1. the default sitemap to use if none is provided in WEB URL
  2. the "inline SVG" feature because it is something that you should enable or disable globally for all devices/browsers, depending only on whether you decided to add or not custom SVG icons containing currentColor as fill color and you want to control the color of these custom icons.

@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch 2 times, most recently from c88e8b3 to fce51f7 Compare February 11, 2024 17:25
@lolodomo
[BasicUI] Move most of app settings to per-device (browser) settings
f3e9e54 
Closes openhab#1832
Closes openhab#1930
Closes openhab#2342

A new page is available to show and update all per-device settings. This page is accessible through the sitemap list page or the home page of any sitemap.
Basic UI theme now matches the dark mode of Main UI. It can be updated either in Main UI or in the new page of Baisc UI.
Web Audio can also be enabled either in Main UI or Basic UI.

The iconify setting is now enabled by default. The script to handle iconify icons is now loaded in sitemap pages only if the iconify setting is enabled.

New settings are also available to control the size of the chart and the size of content (text, line) in the chart.

A new setting is also available to enable a bigger font size.

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo force-pushed the basicui_localstorage_2 branch from fce51f7 to f3e9e54 Compare February 11, 2024 17:29
@lolodomo
Copy link
Contributor Author

lolodomo commented Feb 11, 2024 •
edited
Loading

With bigger font size disabled:

image

With bigger font size enabled:

image

kaikreuzer
kaikreuzer approved these changes Mar 3, 2024
View reviewed changes
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, didn't test it though Thanks for this enhancement!

@kaikreuzer kaikreuzer merged commit e7faba6 into openhab:main Mar 3, 2024
3 checks passed
@kaikreuzer kaikreuzer added this to the 4.2 milestone Mar 3, 2024
@lolodomo lolodomo deleted the basicui_localstorage_2 branch March 3, 2024 09:25
@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 3, 2024

That's cool, thank you for the review.
I am waiting for your feedback after you test it.

@kaikreuzer
Copy link
Member

Seems to work well and it looks pretty nice - thanks again!

stefan-muc
stefan-muc reviewed Jul 12, 2024
View reviewed changes
features/openhab-ui-external/src/main/resources/conf/basicui.cfg

# The number of columns to consider to fill the grid in large screen width (desktop) and medium
# screen width (tablet).
#nbColumns=3-2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sad to see the settings here (especially this one) go. I would like to still have it here to set the default (which then can be overridden by the client). With openHAB 2.4 update, my screens once again show three columns instead of two, and I now have to change it on every screen (some even don't have a mouse connected...) instead of being able to set it at one central location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mueller-ma mueller-ma mueller-ma left review comments

@stefan-muc stefan-muc stefan-muc left review comments

@kaikreuzer kaikreuzer kaikreuzer approved these changes

Assignees
No one assigned
Labels
basic ui Basic UI enhancement New feature or request
Projects
None yet
Milestone
4.2
4 participants
@lolodomo @kaikreuzer @stefan-muc @mueller-ma