feat(UI): project related enhancements#594
Conversation
kubiix
force-pushed
the
dev/feature/project_tweaks
branch
from
0ee107f to
7abfac7
Compare
|
In the meantime, you can squash the fixes and merges into the main commit if you want. |
shpaass
left a comment
shpaass
left a comment
There was a problem hiding this comment.
The line endings in UndoSystem.cs are wrong, which resulted in every line in the the file being shown as changed. After running git add --renormalize Yafc.Model/Serialization/UndoSystem.cs, I was able to look at the actual diff of the changes.
The problem with the star (*) feature is that it depends on the Undo system, and I have questions to the Undo system in general. It's not about your change, but rather about if the Undo system is necessary at all. I'll start the discussion with the team.
The Save and Undo changes are fine concept-wise, but the implementation in the PR is not explained and is not separate from the other changes.
Overall, I reject the changes for the star (*) and ask for separate commits and explanations for the changes about the Undo and Save to continue the review.
After discussing the Undo in #596, we settled on that Undo stays. |
DaleStan
left a comment
DaleStan
left a comment
There was a problem hiding this comment.
I was just recently wanting this *-means-edited title bar feature. I have a couple small changes I'd like to see, though.
| project.saveStateChanged += ProjectSaveStateChanged; | ||
|
|
||
| _ = InputSystem.Instance.SetDefaultKeyboardFocus(this); | ||
| UpdateWindowTitle(false); |
There was a problem hiding this comment.
This call needs to check the undo state. If you specify a file that doesn't exist on the Welcome screen, the title bar doesn't get the *, but you do get prompted to save when you close, even before you close the Milestones panel.
| var projectName = string.IsNullOrEmpty(project.attachedFileName) | ||
| ? LSs.UntitledProject | ||
| : (unsavedChanges ? "*" : string.Empty) + Path.GetFileNameWithoutExtension(project.attachedFileName); | ||
| var title = $"{LSs.FullNameWithVersion.L(YafcLib.version.ToString(3))} - {projectName}"; |
There was a problem hiding this comment.
The document name should come before the program name, so the * doesn't get clipped off when there's limited display space for the string.
There was a problem hiding this comment.
As shpaass mentioned, this file needs to be recommitted with Linux line endings and/or a different local git configuration.
I can take care of that this time, if you'd like.
| // _ = SDL.SDL_SetTextureBlendMode(blurredFade.handle, SDL.SDL_BlendMode.SDL_BLENDMODE_BLEND); | ||
| // _ = SDL.SDL_SetTextureAlphaMod(blurredFade.handle, 160); |
There was a problem hiding this comment.
Are these two lines helpful/necessary?
|
Since the PR-review-requirements were erased during the migration, it seems that I have no option to dismiss my review. Feel free to dismiss it if necessary. |
3 participants
Features
Main window titlenow displays the current project name with * prefix when there are unsaved changesSavein main dropdown is now disabled when there are no unsaved changesUndoin main dropdown is now disabled when there is nothing to undoMisc
SettingsDropdown->MainDropdownandLoadProjectHeavy->ReturnToWelcomeScreen