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

Fix(GUI): Redesign and fix parts of the gui to be more responsive #251

Merged
merged 27 commits into from
Jan 30, 2024

Conversation

simonsasse
Copy link
Contributor

@simonsasse simonsasse commented Jan 24, 2024

Closes #235

TODO

Diff view:

  • Fix save single image offset (Not perfectly fixed..)
  • Move buttons ins app bar from right to left (except help button)
  • Fix size of video frames (4:3 format looks weird)
  • Center frame titles above the frames
  • Move frame titles and buttons closer to frames
  • Reduce fullscreen button size (keep padding of icons in mind)
  • Center timelines horizontally in window
  • Refactor statistical informations - create info icon with title and tooltip (showing infos at hover) - position above the leftmost frame number (above the overview timeline)

Fullscreen view

  • Fix size of frame to be actually full screen
  • Fix size of buttons to be same size as in diff screen

Settings view:

  • Remove back button and implement app bar in settings view - move title to app bar
  • Reduce size of buttons (mask not whole width)
  • More tooltip offset (settings)

Select view:

  • Fix filepath - bigger font size and earlier replacement with dots

General:

  • Center titles horizontally in app bar (all screens)

Remarks from Anton:

Diff Screen

  • the full screen buttons scale too much, there should be a maximum size
  • the icon in the full screen buttons should scale linearly with button size. currently, there is a padding messing with that. It would be nice, if the proportions of the button and the icon always stay the same
  • The Difference Screen headline in the topbar overlaps with the buttons on the left, if the window's width is low enough
  • Save image as png does not pop up where the mouse was clicked (Well not really fixed)

Settings screen

  • I'd suggest putting the save settings button either in the bottom right or int the top bar on the right. Would be more streamlined with other programs' UI

Changes for other PRs

  • Maskpath button not centered (but I guess we'll fix that with the merge of delete mask button #255)
  • Back buttons should ask for confirmation if data would be lost (can be done in a separate PR though)

@simonsasse simonsasse force-pushed the gui/fix/ui-refactor branch 2 times, most recently from e21ac3b to 75531f6 Compare January 26, 2024 10:33
@simonsasse
Copy link
Contributor Author

Merge conflicts resolved.

simonsasse and others added 19 commits January 28, 2024 10:25
Co-authored-by: zino212 <[email protected]>
Signed-off-by: simonsasse <[email protected]>
Co-authored-by: zino212 <[email protected]>
Signed-off-by: simonsasse <[email protected]>
Co-authored-by: zino212 <[email protected]>
Signed-off-by: simonsasse <[email protected]>
Co-authored-by: simonsasse <[email protected]>
Signed-off-by: zino212 <[email protected]>
@simonsasse
Copy link
Contributor Author

Rebased 😓

@simonsasse simonsasse marked this pull request as ready for review January 28, 2024 11:14
@simonsasse simonsasse requested review from a team and AlperK61 and removed request for a team January 28, 2024 11:14
@simonsasse
Copy link
Contributor Author

This should be reviewed by 2-3 people before being merged !!

@akriese
Copy link
Contributor

akriese commented Jan 28, 2024

Great changes :)

Couple things, I noticed:
--- Edit from Luis: Moved remarks into PR description ---

@zino212
Copy link
Contributor

zino212 commented Jan 28, 2024

@akriese: For now I decided to just move the save button into the right corner.

Moving it into the appbar brings possible problems it, because the differentiation between saving and just going back would have to be made very clear. I don't know if positioning the button on the left next to the back button would be better or positioning it to the right. On the left the user would probably be more likely to see it than on the right but maybe it being so close to the back button would also be problematic.

In less words: The button being in the lower right corner is in my opinion for now the solution with the least problems. :)

@zino212
Copy link
Contributor

zino212 commented Jan 28, 2024

The scaling of the full screen button seems to be a bit tricky. I set a maximum size for the vertical size for now. The horizontal size is responsive to the overall window size and therefore can not be set this way.
It would probably need a bigger refactoring to set that.

Additionally, I have no real clue on how to set a minimum size for the icons or how to make them scale linear with the buttons. I tried several things which didn't work out. Maybe @simonsasse has some ideas?
Regarding the overlapping title my ideas didn't work out as well. The borders of the different areas of the appbar seem to be kind of messed up. When scaling the window down, the title seems to notice the navigation area, but far to late. The save image button seems to be placed out of the regular navigations button area and therefore the title overlaps.

@simonsasse
Copy link
Contributor Author

OKaaaay.
I made the appBar responsive to an extend. very narrow windows still overlap. But I think for what's relevant, it's fine now. Buttons get collapsed into a dropdown menu.

I changed the fullscreen button to be a plain IconButton. Looks cleaner and does not scale ugly.

I centered the download dropdown on the image. I am sure, it is possible to get the exact location and place it there but maybe that's not the most relevant thing for now.

@akriese akriese self-requested a review January 29, 2024 15:06
Copy link
Contributor

@akriese akriese left a comment

Choose a reason for hiding this comment

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

All in all, this LGTM. Let's wait for at least one more positive review though :)

@simonsasse simonsasse merged commit 4fc36f6 into main Jan 30, 2024
8 checks passed
@simonsasse simonsasse deleted the gui/fix/ui-refactor branch January 30, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve design with Material 3
3 participants