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

Normalization doesn't move the viewport (#394) #395

Conversation

Atria1234
Copy link
Collaborator

@Atria1234 Atria1234 commented Dec 18, 2021

  • LayoutVersion is loaded when layout is loaded

  • Change of layout version is undoable and therefor counts toward unsaved changes

  • Star is shown in the window title when there are unsaved changes

  • Check zoom and selection option for export

@FroggieFrog
Copy link
Collaborator

Don't forget to check the export of an image, because Normalize doesn't change the Viewport anymore.
Maybe other functions also depend on it.

@Atria1234
Copy link
Collaborator Author

Export works, but good point to check other functionality. For example opening a file should still reset the viewport to top left.

@Atria1234 Atria1234 force-pushed the feature/dont-move-viewport-during-normalization branch from 8e4f807 to be663b6 Compare December 23, 2021 15:04
@FroggieFrog FroggieFrog added this to the Anno Designer 9.3 Release milestone Dec 27, 2021
@FroggieFrog FroggieFrog linked an issue Dec 27, 2021 that may be closed by this pull request
@FroggieFrog
Copy link
Collaborator

FroggieFrog commented Jan 13, 2022

Some minor things I found during testing:

  • after using Save as the layout version is not shown in the Title of MainWindow
  • After using Save as it seems that the saved layout is loaded instead of keeping the original layout open. Is this intended? I think the default behavior of other apps is different.
  • after using Tools->Normalize the Title has an asterisk even if there are no changes in the layout

@Atria1234
Copy link
Collaborator Author

I couldn't reproduce layout being loaded after the SaveAs action, but there were some inconsistent behaviour which now should be fixed

@FroggieFrog
Copy link
Collaborator

@Atria1234 Could please rebase/merge with master? Currently it is way to messy for me to distinguish between changes in this PR and changes made in other PRs.

Also I missed options to specify if if current zoom and current selection should be used on exported image. Maybe those are missing options in CanvasRenderSetting and should be checked in MainViewModel.cs lines 1436 and 1442?
Those options can be set via the Export menu.

Atria1234 and others added 5 commits January 17, 2022 16:03
- LayoutVersion is loaded when layout is loaded
- Change of layout version is undoable and therefor counts toward unsaved changes
- Star is shown in the window title when there are unsaved changes
- rename converter in xaml to match existing variables (formatting is automatically done by EditorConfig)
- use `EventHandler<T>` instead of custom delegate
- version didn't update after Save action
- version didn't show up after SaveAs action
@Atria1234 Atria1234 force-pushed the feature/dont-move-viewport-during-normalization branch from 832bc02 to 559ee0b Compare January 17, 2022 15:20
@Atria1234
Copy link
Collaborator Author

I'll just rebase now, I'll check the zoom and selection export setting later

@Atria1234
Copy link
Collaborator Author

Could you try to reproduce the problem with exporting current zoom and selected objects? Both seems to be working for me

@FroggieFrog
Copy link
Collaborator

@Atria1234 Could you please double check if this still works as intended after my resent merge with master?

@Atria1234
Copy link
Collaborator Author

Works, except when I tried to check the merge I noticed that MainWindow.WindowClosed is no longer called on Window's Closed event. Is that wanted behaviour? When I added it back it caused stack overflow exception, but stack was 4 level high, so I don't know what that causes.

@FroggieFrog
Copy link
Collaborator

The Closed event got lost during merge.
I also get the exception, but only when a debugger is attached. Is this a problem?

@Atria1234
Copy link
Collaborator Author

Atria1234 commented Jun 17, 2022

I don't know, you added that event 😄 I guess it is a problem when the "another app running" window gets shown as part of the startup.

But as far as this only happening when debugger is attached, I'd say it is pretty annoying. But does there even have to be the base.OnClosed? This is an event handler, not overriden OnClosed.

@Atria1234 Atria1234 force-pushed the feature/dont-move-viewport-during-normalization branch from e156ee3 to 4e2a983 Compare June 18, 2022 04:29
@Atria1234 Atria1234 force-pushed the feature/dont-move-viewport-during-normalization branch from 4e2a983 to 58011e8 Compare June 18, 2022 04:32
@FroggieFrog
Copy link
Collaborator

I'm sorry, but if I'm merging it with the latest master it does not work. The viewport is always reseted.
Could you please merge it with master (again) and check the behavior is correct?
This PR will be merged next. I promise 😄

@Atria1234 Atria1234 force-pushed the feature/dont-move-viewport-during-normalization branch from e78f384 to 135701c Compare June 21, 2022 13:27
@Atria1234
Copy link
Collaborator Author

I deleted the OnClosed event from MainWindow, since the shutdown mode is reset after the "multiple instance running" message is shown.

@FroggieFrog
Copy link
Collaborator

FroggieFrog commented Jun 22, 2022

It could be that I understand this PR wrong, but this is the current behavior of this PR:

  • ViewPort is reset after Normalize
  • ViewPort is not reset after Save

normalize

@Atria1234
Copy link
Collaborator Author

Yes, that is expected behaviour. The issue I had with normalization after save was just that...if you were working on some south part of CF, saving would reset your viewport to the top left.

Explicit requestion normalization still resets the viewport. Do you think that viewport should not be reset after requesting normalization?

@FroggieFrog
Copy link
Collaborator

Ok, so I got this PR wrong.
The problem was that the ViewPort was changed during Save.
I thought because of the title (Normalization doesn't move the viewport) the Normalize action was the problem.
So a changlog could say:

The view of the layout is no longer shifted around when saving.

I think leaving the behavior of Normalize as it is right now is the right thing to do.

@FroggieFrog FroggieFrog merged commit ca80b25 into AnnoDesigner:master Jun 23, 2022
@Atria1234 Atria1234 deleted the feature/dont-move-viewport-during-normalization branch June 23, 2022 13:03
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.

Viewport is moved to top left corner after save
2 participants