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

feat: split off the current image in iv into a separate window (#2058) #4017

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

antond-weta
Copy link
Contributor

Description

This change adds a menu item "Split off" under the menu "File" which creates a new window and initialises it with the currently displayed image.

Tests

I have tested the UI changes on Mac and linux, I don't have a Windows machine currently.
No other functionality should be affected by this change.

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • If I added or modified a C++ API call, I have also amended the
    corresponding Python bindings (and if altering ImageBufAlgo functions, also
    exposed the new functionality as oiiotool options).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 12, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@antond-weta
Copy link
Contributor Author

The new window gets initialised with the default settings. I'm not sure how much of the current window's viewer state we want to retain.

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

Nice! Instead of "split off", I would suggest calling the menu item "Move to new window", that seems to be what the equivalent operation is called in some other apps I just randomly sampled (Firefox, Chrome, Obsidian).

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

I suggested a different name for the command, but other than that, this LGTM.

@antond-weta
Copy link
Contributor Author

I suggested a different name for the command, but other than that, this LGTM.

Thanks Larry, I have renamed the command.
There are currently some EasyCLA issues happening on our side, they are being chased with LFX.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM! Just awaiting CLA to merge.

Note that #2058 was an aggregation of many scattered issues. This suggestion for this feature originally came from #287, filed in 2012!

@lgritz
Copy link
Collaborator

lgritz commented Oct 19, 2023

Aha, your CLA is working now!

@antond-weta, there appear to be a couple very minor clang-format fixes needed. As soon as that's ready, we're definitely ready to merge this.

…fix formatting

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>
@lgritz lgritz merged commit e51dd5c into AcademySoftwareFoundation:master Oct 19, 2023
26 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Oct 20, 2023
…cademySoftwareFoundation#4017)

This change adds a menu item "Move to new window" under the menu "File" which
creates a new window and initialises it with the currently displayed
image.

I have tested the UI changes on Mac and linux, I don't have a Windows
machine currently.
No other functionality should be affected by this change.

This was one item from issue AcademySoftwareFoundation#2058 

---------

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>
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.

None yet

2 participants