-
Notifications
You must be signed in to change notification settings - Fork 662
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
Increase amount of action buttons #2903
Conversation
Many thanks, this looks reasonable. Could you show some screenshots form before your changes? I was actually wondering how to enable all those buttons. I can add one to create still images (when enabled), but the others are, I guess, added depending on the camera device, whether it supports zooming, moving, a light, or other functions? Previously only the camera name and motionEye buttons are at the top, the framerate at the button left and the action buttons at the bottom right. It makes sense that they can exceed the camera frame, or do some ugly line break when being too many. I wonder whether it makes sense to leave the camera name larger in one row, and the framerate and optional monitoring info in two rows, like it was before? For identifying reasons, the camera name might better large. Or alternatively, probably the best solution, have the framerate below the camera name, as it is always available, and the monitoring info as optional second column. That way also on very small camera frames, if no monitoring info is shown, there is more space for all info at the top. It is a different topic, but in my case, and I see the same in one of your cases, the monitoring info is an empty string, and shown as Side note: The CodeFactor annotations are unrelated to this PR and, AFAIK, wrong, since those functions/variables are in fact used from HTML elements. I tested the PR as patch, and it works as intended. One needs to do a forced-reload to load the new frontend code. We really need to add some versioning for this, but a different topic. |
I indeed meant a "column", not a "row" what you added now. So basically switching framerate and monitoring info positions form how the PR was at first, and then have this 2nd column removed if no monitoring info is available. But actually I like the solution with a 3rd row more, respectively a 2nd row considering how elements are nested. Allow text information listed below each other IMO is clearer, and I think this little additionally removed/overlaid camera view is worth it. So from my end: Let's keep what you have now 👍. Thanks for addressing the CodeFactor annotations. Exactly how I'd have done in a dedicated PR. About the layout rows: I remember we recognised that there is not really a point to allow defining layout rows and columns both, and that even changing the amount of rows did not have any effect on tests: It is sufficient to just allow defining the amount of columns (camera frames per line) and the amount of rows is implicitly defined by the amount of cameras instead. I thought we did some change in this regards already, but could not find the PR. So I guess this part was broken in 0.42.1 already. Currently, the GUI settings still allow to define the amount of rows. Do you mean, while this is possible, and indeed changes the |
So from my standpoint I think everything is completed so far. If you have any other ideas / improvements I'm happy to implement it. Concerning you points about the CodeFactor issues, thank you for clarifying this. I would like to do this properly to check for any leftovers, not just delete the blocks I commented out. As soon as this PR is merged, I will do a proper cleanup and open a new PR. I really want to check where all those variables are used and what can be removed without removing too much. 😅 |
Great work! Let's do the cleanup right away. We have this PR as reference and the commits you referenced for the obsolete variables/functions remain visible here. I'll do a final test this weekend and merge it if everything works well. |
I moved the calculation for the action buttons to a separate callback function which only gets executed when the layout updates. Before it was executed once a second which is quite unnecessary. But now really everything is done, so you can test it. Thank you. |
Hi, could you do the testing and merge this weekend, so I could to the cleanup in the next step as we talked about? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks and works all as expected. Many thanks!
Hi
I made some UI changes to increase the available action buttons. There have been several requests concerning this. This pull request fixes: #613 ,#844, #1766.
I moved the frame rate and monitoring info to the top. I tried to optimize everything for space constraints. There are two rows in the top left. First one for camera name and frame rate and the second one for monitoring info. If there is no monitoring info, the second row disappears and name and fps are centered. The bottom is now solely for action buttons. Instead of 8, it allows up to 16. If there are only a few or if the window is wide enough, there is only one row. If more are added or the view changes and the camera window gets smaller, a second row appears.
There are no breaking changes or anything, simply a UI enhancement. The only thing I am not super happy with for now is that in order to be able to make the buttons evenly spaced ( I used flex), I need to check the width of the camera window and make the settings in css accordingly. To do so, I check on a regular basis for the width. The only place I have found so far is in the main.js where I found a function that get's executed once a second to update frame rate and stuff. This is the main code for on line 5096 in main.js:
But I don't need to check it regularly, only when the UI changes. There is this function "updateLayout();" in main.js, which gets called only when the view changes but I don't see a way to access the individual data of each camera-window inside this function. Any recommendations are welcome. But I think for now, this isn't to big of a problem.