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 change of output path #21

Closed
wants to merge 0 commits into from
Closed

Allow change of output path #21

wants to merge 0 commits into from

Conversation

dz0ny
Copy link
Contributor

@dz0ny dz0ny commented Apr 27, 2023

Adds new UI elements that allow to change and preview output path.

image

Does #11

@dz0ny dz0ny force-pushed the master branch 4 times, most recently from 08c6897 to 73810d5 Compare April 27, 2023 19:28
@avsaase
Copy link
Owner

avsaase commented Apr 27, 2023

Hi, thank for the PR! I will review in in the next few days.

@dz0ny dz0ny force-pushed the master branch 5 times, most recently from 285fbb0 to bebd964 Compare April 28, 2023 18:24
Copy link
Owner

@avsaase avsaase left a comment

Choose a reason for hiding this comment

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

I took a closer look at this and I'm not sure I like how this looks UI-wise. The empty or filled circles next to the input files are meant to indicate if the respective files are loaded but this doesn't make sense for the output path. Maybe adding a output path text box at the bottom of the screen similar to what Gyroflow has would be better?

There are also some other issues:

  • The default width of the side bar is not large enough to fit the output file name.
  • The output file name is formatted as a link but this doesn't work if the file doesn't exist. If it does exist for me it opens the video in my browser. I would rather have a button that opes the system's file explorer focused on the out put file.
  • Saving the output path to the settings file should be optional, or at least the output path should be configurable. I organize my flight videos in folders named with the date of the flight so personally I want the videos to be rendered in the same folder as the input files.
  • When there is no output path set in the saved settings file and you load a video, the path is not shown. It is saved to the file so when the app restarts it is shown, giving an inconsistent experience.

You're more than welcome to address these issues but I want to let you know that I'm considering switching from egui to Iced for the GUI. The first step is properly decoupling the UI and backend code and moving it into separate crates. This is already a pretty big diff and once I get to changing the UI code it'll be much bigger still. Merging in your changes later might become tricky and perhaps not worth it if the UI is redesigned anyway. I'll leave that decision up to you.

@dz0ny dz0ny closed this Jun 21, 2023
@dz0ny dz0ny force-pushed the master branch 2 times, most recently from bebd964 to 9338c21 Compare June 21, 2023 19:38
@dz0ny dz0ny mentioned this pull request Jun 21, 2023
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.

2 participants