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

allow multiple layout columns on mobile #1570

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

digiltd
Copy link

@digiltd digiltd commented Dec 15, 2019

Having a single column view with multiple cams on mobile/tablet was quite restrictive, better to give user the choice?

Having a single column view with multiple cams on mobile/tablet was quite restrictive, better to give user the choice?
@ccrisan
Copy link
Collaborator

ccrisan commented Dec 15, 2019

The reason why those CSS lines were added is that when I visit motionEye from desktop, I'd like to have multiple columns, when dealing with multiple cameras, but on mobile I prefer seeing them in one single column.

While I agree that we should give the user a choice, I don't think this is the correct solution. Unfortunately, preferences are stored per user and apply to all devices.

@digiltd
Copy link
Author

digiltd commented Dec 16, 2019

Fair enough, for me I use a tablet stuck to the wall in my office which can comfortably display 2-3 cols.

Thoughts on storing the cookie for the layout in user's local storage? Or if you prefer to keep all preferences on the server, having options for both mobile and desktop?. Though I think the latter would get unnecessarily complicated.

@ccrisan
Copy link
Collaborator

ccrisan commented Dec 16, 2019

Storing prefs as cookies would be indeed the correct solution here.

@MichaIng
Copy link
Member

Any news here? Wouldn't it make sense to do this automatically based on screen/window width instead or hardcoding it per mobile vs desktop (user agent?)? E.g. also on a small mobile phone in horizontal view one might again want to have multiple columns.

@digiltd
Copy link
Author

digiltd commented Mar 18, 2022

It was a while ago, I think at the time (and seeing what I was trying to change) it forced the one col view to anything below 1200px, keeping the user configurable rows/cols options only for screens above 1200px.

As 1200px is not that small, I thought it was more beneficial to allow the user to choose to have a row of 5 camera views on a 400px wide device if that is what they really wanted.

@MichaIng
Copy link
Member

Generally I agree, though the default view should be reasonable and the 1200px break point doesn't seem to bad.

Actually I have to see how the prefs menu that was hidden and shown with your change look like, I have not recognised it yet. Probably we can show it in all cases (if it is not too tall, probably hiding it on very small screens) and only have a different default.

@MichaIng
Copy link
Member

MichaIng commented Mar 18, 2022

Okay found it. Not sure how to solve it best. I personally do not like, with this change, that even on tiny mobile screens one sees three columns by default and needs to find the preferences first to adjust it.

The break point cannot be made variable so easily, not sure whether we can set an inline CSS variable with the backend and use this in the frontend (with 1200px being the default).

Another option would be to have dedicated preferences for large and small screens, while only the slider for the current screen size is shown to not have too many confusing settings shown at the same time.

OOT

Btw, does the Fit Frames Vertically + Layout Rows settings have any effect for you? They seem to be not working at all in my case, only the columns count does. I was testing with dev branch, so if this is still working with latest stable release, then we'd need to fix it in dev, else device whether to fix or simply remove it, since it should break the aspect ratio anyway or create overscan or underscan (camera frame with black borders left and right, or being larger than the browser window).

@zagrim
Copy link
Collaborator

zagrim commented Mar 19, 2022

Btw, does the Fit Frames Vertically + Layout Rows settings have any effect for you? They seem to be not working at all in my case, only the columns count does.

In my production setup (ME 0.41rc1 - I should have upgraded a long ago, but it works why touch it 😉 ) "Fit Frames Vertically" does work. If I set the browser window size such that the frame (or row of frames) is taller than the available vertical space, with that setting enabled the frame gets scaled down so that it fits the window height.

Layout Rows OTOH doesn't seem to do anything. But since the layout seems to work so that it first lays frames out horizontally and after that vertically, and it must show every camera, I wonder how could it work as described in the tooltip... But I have only two cams in the setup, so I might not be able to get the particular use case it is designed for.

@MichaIng
Copy link
Member

Verified it works as described by @zagrim with Fit Frames Vertically shrinking frames to always show one fully in browser window, but Layout Rows without any effect that I could find so far.

However, here it is about horizontal layout choice for windows <=1200px, sorry for mixing topics 😉. The question remains whether to:

  • Allow changing the break point
  • Allow changing layout columns for <=1200px screen separately, which defaults to 1 instead of 3 but can otherwise be freely changed as well
  • Make it a per-browser setting, stored in the browsers local store, which is initialised with 1 or 3 based on window width. But AFAIK this has always a limited life time, i.e. one needs to re set after it timed out. Not sure whether in theory it can be set to very long or unlimited life time.
  • A cookie could be used as permanent per-browser setting, but it doesn't fit for a layout setting since cookies are sent to the server which is unnecessary here where related data can perfectly fine remain in browser only.

@zagrim
Copy link
Collaborator

zagrim commented Mar 20, 2022

Make it a per-browser setting, stored in the browsers local store, which is initialised with 1 or 3 based on window width. But AFAIK this has always a limited life time, i.e. one needs to re set after it timed out. Not sure whether in theory it can be set to very long or unlimited life time.

Local storage has limited life? If we're talking about browser local storage, I think it should persist just as long as cookies, which means until the user does some clean-up action (https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API). But there seem to be all kinds of storage (https://developer.mozilla.org/en-US/docs/Web/API/Storage_API) and e.g. IndexedDB API talks about quotas and LRU policies, but I think that's not relevant to the key-value pair type of local storage which I guess would be applicable here.

@MichaIng
Copy link
Member

MichaIng commented Mar 20, 2022

Probably I'm wrong about the life time because I know its usage from JWTs which are probably externally invalidated. The Web Storage API's localStorage seems exactly what we'd need indeed. To me this makes much more sense, leaving these layout settings entirely on the client, than storing such on the server and passing it around all the time. But that would be a little larger rework 🙂.

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

Successfully merging this pull request may close these issues.

None yet

4 participants