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.

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

[Basic UI] On narrower screens Chart takes more place #1930

Closed
dilyanpalauzov opened this issue Jun 17, 2023 · 16 comments · Fixed by #2243
Closed

[Basic UI] On narrower screens Chart takes more place #1930

dilyanpalauzov opened this issue Jun 17, 2023 · 16 comments · Fixed by #2243
Labels
basic ui Basic UI bug Something isn't working

Comments

@dilyanpalauzov
Copy link

Below are two screen shots from browser. In the second one the width of the browser is smaller, but the chart is bigger. This is against every logic.

I suggest either using always the larger variant of the chart, or using for the chart one column, and in the other column inserting several “normal” items, so that no vertical space is left unused.

image

image

@dilyanpalauzov dilyanpalauzov added the bug Something isn't working label Jun 17, 2023
@lolodomo
Copy link
Contributor

lolodomo commented Jun 22, 2023

It depends if you have one or two columns.
The built chart has probably a certain width, not too big to get a reasonable time rendering.
And I believe Basic UI does not apply image upscaling.
I can check to be sure of the correct answer.

Your suggestion is very probably too much complex to implement.

@lolodomo
Copy link
Contributor

You have chart options to define the size of your charts. By default, size is 480x240. You can increase these settings as you like.

image

@lolodomo
Copy link
Contributor

lolodomo commented Jun 22, 2023

Try to set 960x480 for example and you will have a good surprise.

If you have a good server CPU, it should be fine. Even with a RPI, it is still reasonable.

@lolodomo lolodomo added the basic ui Basic UI label Jun 22, 2023
@lolodomo
Copy link
Contributor

My answer should have resolved your "issue".

@lolodomo
Copy link
Contributor

You can fill a feature request if you would like a different behaviour.
PS: I will not spend dev time myself on that.

@dilyanpalauzov
Copy link
Author

Try to set 960x480 for example and you will have a good surprise.

I mean out of the box this does not work good, if this has to be set up.

I do not see why it shall be complex to implement that a column (and charts in column) always has the same width, irrespective of how many columns are displayed.

@lolodomo
Copy link
Contributor

I mean out of the box this does not work good, if this has to be set up.

BasicUI choice was to not upscale images.

And default size for charts was defined to get a fast chart creation even with a small hardware like a RPI.

I do not see why it shall be complex to implement that a column (and charts in column) always has the same width, irrespective of how many columns are displayed.

What looks complex to me was your suggestion "and in the other column inserting several “normal” items, so that no vertical space is left unused.".

Maybe a good "compromise" would be to upscale charts, keeping normal images unscaled.

As the change is simple for you, you can always propose a change through a PR and we will review your proposal.

@lolodomo
Copy link
Contributor

lolodomo commented Aug 4, 2023

Maybe a good "compromise" would be to upscale charts, keeping normal images unscaled.

I reopen the issue.
In fact, there is already an upscale when the display is on one column but not when the display is on two columns. It is not very consistent.
I will make it consistent, either by upscaling in both cases or by adding a button to allow switching between unscaled and upscaled. .

@lolodomo lolodomo reopened this Aug 4, 2023
@lolodomo
Copy link
Contributor

lolodomo commented Aug 5, 2023

Image and chart are upscaled as soon as the screen width is <= 850 pixels. That is on a desktop. I am not sure what was the reason to introduce this different behaviour.

What I am not yet understanding is why upscale is systematically applied on my phone while its screen width is in principle 1080.

@lolodomo
Copy link
Contributor

lolodomo commented Aug 5, 2023

What I am not yet understanding is why upscale is systematically applied on my phone while its screen width is in principle 1080.

If I comment our CSS code triggering the upscale of images when screen width is <=850 pixels, I can see that upscale is no more applied, neither on desktop, nor on phone. That would mean that my phone screen width is detected as <= 850, that is strange! Is it possible on an Android phone to check what is the current screen resolution (like on Windows) ?

My plan for image (and chart) is to disable the upscale for small screen resolution but provide a button in a new image header row to switch between unscaled and upscaled.

@lolodomo
Copy link
Contributor

lolodomo commented Aug 5, 2023

My plan for image (and chart) is to disable the upscale for small screen resolution but provide a button in a new image header row to switch between unscaled and upscaled.

For charts, we could decide if the default should be "upscaled" or not. The size of the chart produced by the OH server is driven by OH settings, the default is 240x480. Upscaling such resolution is reasonable on a phone but leads to bad quality image on a big desktop screen.
Changing the default chart resolution will lead to more time to generate the chart, that would be sensible on very small hardware. But 480x960 seems to work well on my RPI3. I will check the approximative time difference between 240x480 and 480x960 and propose a change in case the delta is reasonable. 240x480 was probably chosen for RPI first generation.

@lolodomo
Copy link
Contributor

lolodomo commented Aug 5, 2023

That would mean that my phone screen width is detected as <= 850, that is strange!

That is finally clearer for me after few more WEB readings. Each device has a screen resolution and a Density Pixel Ratio, that means a physical resolution and a logical resolution. Pixels in CSS are based on logical resolution.
To get Android screen resolution and DPR: http://whatismyandroidversion.com/
It confirms my phone physical screen resolution of 1080x2340. And I see a DPR of 2.75.
So that leads for WEB browsers to a logical approximative resolution of 392x850.
Now I better understand the threshold defined in Basic UI for tablet (850) and phone (440).
It explains why I see image upscaling on my phone, the trigger is setup at 850 in the WEB app.

@lolodomo
Copy link
Contributor

lolodomo commented Aug 6, 2023

Changing the default chart resolution will lead to more time to generate the chart, that would be sensible on very small hardware. But 480x960 seems to work well on my RPI3. I will check the approximative time difference between 240x480 and 480x960 and propose a change in case the delta is reasonable. 240x480 was probably chosen for RPI first generation.

I tested with my RPI 3 and the time to render a 240x480 or 480x960 chart is very similar. What impacts the rendering time is in fact not really the chart resolution but the number of data you have in your chart (time frame & number of items).
The problem with a chart resolution of 480x960 is that the texts are very small on a phone with high resolution screen in portrait mode.
It leads to something else that would be a prerequisite before changing the default chart resolution, that is setting the "dpi" parameter of XChart library. This parameter is already managed by the OH core chart servlet when provided, but it is not set in Basic UI. By setting it in Basic UI in particular for phones with high DPR, I could expect to have less small texts when using 480x960 chart resolution.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 8, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image.
Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image, chart, video, mapview and webview elements, if the label is
emppty, the header line is hidden.

For chart and image elements, there is now no upscale applied by
default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 11, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image.
Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image, chart, video, mapview and webview elements, if the label is
emppty, the header line is hidden.

For chart and image elements, there is now no upscale applied by
default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 11, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image.
Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image, chart, video, mapview and webview elements, if the label is
emppty, the header line is hidden.

For chart and image elements, there is now no upscale applied by
default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 16, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to switch between no upscale and upscale of the image. Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and webview elements.

For image and chart elements, the header line is always present so that user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the header line is hidden.

For chart and image elements, there is now no upscale applied by default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 16, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to switch between no upscale and upscale of the image. Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and webview elements.

For image and chart elements, the header line is always present so that user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the header line is hidden.

For chart and image elements, there is now no upscale applied by default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Aug 16, 2023
Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to switch between no upscale and upscale of the image. Header line for chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and webview elements.

For image and chart elements, the header line is always present so that user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the header line is hidden.

For chart and image elements, there is now no upscale applied by default (tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
kaikreuzer pushed a commit that referenced this issue Sep 9, 2023
…2010)

Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image. Header line for
chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image and chart elements, the header line is always present so that
user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the
header line is hidden.

For chart and image elements, there is now no upscale applied by default
(tablet/phone devices) but a button allows upscaling.

Closes #1939
Fixes #1367
Also related to #1930

Signed-off-by: Laurent Garnier <[email protected]>
stefan-hoehn pushed a commit to stefan-hoehn/openhab-webui that referenced this issue Sep 23, 2023
…penhab#2010)

Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image. Header line for
chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image and chart elements, the header line is always present so that
user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the
header line is hidden.

For chart and image elements, there is now no upscale applied by default
(tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Stefan Höhn <[email protected]>
JustinGeorgi pushed a commit to JustinGeorgi/openhab-webui that referenced this issue Sep 24, 2023
…penhab#2010)

Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image. Header line for
chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image and chart elements, the header line is always present so that
user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the
header line is hidden.

For chart and image elements, there is now no upscale applied by default
(tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: jgeorgi <[email protected]>
JustinGeorgi pushed a commit to JustinGeorgi/openhab-webui that referenced this issue Sep 24, 2023
…penhab#2010)

Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image. Header line for
chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image and chart elements, the header line is always present so that
user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the
header line is hidden.

For chart and image elements, there is now no upscale applied by default
(tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: jgeorgi <[email protected]>
digitaldan pushed a commit to digitaldan/openhab-webui that referenced this issue Sep 24, 2023
…penhab#2010)

Header line for video element contains icon and label.

Header line for image element contains icon, label and a button to
switch between no upscale and upscale of the image. Header line for
chart element contains icon, label and 4 buttons:
 - one button to show or hide the legend
 - one button to change the time range
 - one button to switch between no upscale and upscale of the chart
 - one button to refresh the chart

Fix handling of iconcolor and labelcolor parameters for mapview and
webview elements.

For image and chart elements, the header line is always present so that
user has an access to its buttons.
For video, mapview and webview elements, if the label is empty, the
header line is hidden.

For chart and image elements, there is now no upscale applied by default
(tablet/phone devices) but a button allows upscaling.

Closes openhab#1939
Fixes openhab#1367
Also related to openhab#1930

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo
Copy link
Contributor

As I plan to add per device settings for Basic UI, I will also add a new DPI setting that will be used for chart, I guess something similar to the existing setting in the Android app.

@lolodomo
Copy link
Contributor

lolodomo commented Jan 30, 2024

The current default chart dimension is a compromise, probably too big for a phone, ok for a tablet and too small for desktop.

I will add another device setting to choose amongst predefined chart dimensions. I am working on it.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Feb 7, 2024
Fix openhab#1832
Closes openhab#1930

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.

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

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Feb 7, 2024
Fix openhab#1832
Closes openhab#1930

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.

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

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Feb 7, 2024
Fix openhab#1832
Closes openhab#1930

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.

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

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo
Copy link
Contributor

lolodomo commented Feb 7, 2024

Regarding default chart size, I think the current default size is a good compromise. Of course, it is already too big for a phone, meaning the image will be downscaled, and it is too small for a big desktop screen.
With my new PR (settings at device browser level), you can now select a (predefined) size that will be specific to your device browser. You will then select a small size on a phone and a big size on a desktop.
I also provided a setting to adjust the chart DPI parameter. It can be used to partially compensate the downscaling applied on a phone, but it is clearly a better option to choose an appropriate chart size.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Feb 8, 2024
Fix openhab#1832
Closes openhab#1930

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.

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Feb 11, 2024
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 added a commit to lolodomo/openhab-webui that referenced this issue Feb 11, 2024
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basic ui Basic UI bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants