ExportDialogDeclarative2

[cmeyer]

• Refactored Export Dialog in Declarative Mode
• Do not pass redundant/unused 'parent_window'. Do not assign export dialog to unused variable.
• Clean up instance variables in export dialog and its model.
• Ensure that correct writer index is initialized.
• Change export filename building to use pathlib.
• Improve dialog box with default width and stretches.
• Ensure selected writer is saved in preferences when export is pressed.
• Fix issue where filename has a '.' (isodate on mac/linux).

[cmeyer]

@Tiomat85 Thanks for the PR! Much improved. In the interest of moving your PR forward, I made a number of changes to clean up the code and fix bugs. By the time I was through I had made more changes than expected, so I decided to create a new PR with explanations of the changes. I squashed your changes, then made several commits on top of yours, described in order below.

If you're ok with these changes, approve this PR (or if not, make comments) and I'll merge asap.

0c7513b self explanatory minor changes

335800e the main thing here was to (a) eliminate "optional" instance variables. now writer is always assigned in the ExportDialogViewModel.__init__ method. Optional instance variables can lead to bugs down the road. As a general rule, I try to assign to all used variables in __init__ so we always know they're present (lest you end up with AttributeErrors when inadvertently accessing something that hasn't been assigned yet. In addition to that, I removed redundant writer and directory instance variables on the ExportDialog class, which are available unambiguously on the viewmodel.

a631cb1 the existing PR didn't initialize file_type_index (now called writer_index) with the correct index to match the writer. So the user could think they were exporting as JPEG (the first writer) but in actuality they would export with PNG (as assigned to writer). this fixes that.

6991f2f changes the build_filepath to use the pathlib Python module, which is more modern and easier to use. In addition, assumes now that directory_path exists before calling. Also renames to use "file path" in place of "file name" where appropriate.

c4bc0e3 minor cleanup to make the dialog look nicer. Still might need some more improvement (such as eliding the directory name, and making the 'prefix' select automatically). Note, along the way I fixed an issue where the file format combo box was bound to file_type_index.value rather than file_type_index (now called writer_index). also changed the "prefix string" to use create_line_edit (single line edit) rather than create_text_edit (multi-line edit).

6714529 ensure that the last exported file type preference is saved.

557c22a fixes a bug introduced by my pathlib changes, but I left it in here to be instructive. in this case, the isodate gives a string with a period on some platforms; then when replacing the extension, it will truncate at the period and add the suffix. this change just replaces '.' with '_' which is probably what is wanted in most cases anyway.

More items to-do (after merge):

• elide the directory string to fit to width. the UserInterface.py truncate_string_to_width may be helpful.
• clean up the prefix string to auto-enable if user enters prefix string
• save the other preferences from viewmodel. the UserInterface.StringPersistentModel and similar classes may be helpful here.

[cmeyer]

@Tiomat85 Is it ok to merge this? (I saw the other PR was closed)

[Tiomat85]

Sure, I had a quick look through some of it and it looked to be tidying up variables that weren't needed that hung around from the previous version and my lack of understanding of some of the structures. The only outstanding question would be the extra little issue-ettes you mentioned that are worth doing post-merge, are they going to be created as their own individual Issues? That would make the most sense to me as they are not directly blocking the original issue from being completed, so better to get that *done* and any follow up work that we discover makes sense as brand new work items to be done at some point in the great future. p.s. Trying the email reply option from github so this may not come through clearly.

…

On Wed, Jun 12, 2024 at 4:14 PM Chris Meyer ***@***.***> wrote: @Tiomat85 <https://github.com/Tiomat85> Is it ok to merge this? (I saw the other PR was closed) — Reply to this email directly, view it on GitHub <#1065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKEJZNLV2TBOMY3T346I7TZHBQUNAVCNFSM6AAAAABJFDUFVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRTGI4TKNRYGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cmeyer]

Put smaller issues here:

• Export batch with no boxes checked overwrites same file repeatedly #586