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.

Sign up for GitHub

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

Jump to bottom

Fix undo (CTRL + Z) and redo for inserting an entry #9777

Merged
merged 9 commits into from
May 7, 2023
Merged

Fix undo (CTRL + Z) and redo for inserting an entry #9777

merged 9 commits into from
May 7, 2023

Conversation

brunaoo
Copy link
Contributor

@brunaoo brunaoo commented Apr 18, 2023 •
edited by ThiloteE
Loading

Fixes #9737

Removed default switch from EditAction class. This was causing an exception for the user when the Ctrl + Z command was issued.

Compulsory checks
Beta Give feedback

@koppor
Copy link
Member

koppor commented Apr 18, 2023

Thank you for working on it. Please check your checkstyle setup at https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace. Scroll down...

@brunaoo
Copy link
Contributor Author

brunaoo commented Apr 18, 2023

Thank you for working on it. Please check your checkstyle setup at https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace. Scroll down...

OK!

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 18, 2023
@koppor
Copy link
Member

koppor commented Apr 18, 2023

For a quick fix, you can click on "Files" above and see Review Dog complaining about two spaces (which should be one). This should be an easy fix ^^.

@calixtus
Copy link
Member

If the bug was introduced in 5.10 it is not necessary to document the fix in the changelog, since we only mention fixes from release to release:
grafik

However, this fix may remove the exception, but the problem is not fixed.
You killed the messenger, but did not fix the issue itself.

The real question is imho: Why is the Ctrl-Z keyboard event not consumed, but reaches the switch statement?

@koppor
Copy link
Member

koppor commented Apr 18, 2023

There should be a GUI test - if possible. There are other GUI tests at https://github.com/JabRef/jabref/tree/main/src/test/java/org/jabref/gui.

@Siedlerchr
Copy link
Member

We took another look at this PR and found at least one root cause for the issue and at least input controls work now

@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 24, 2023
@ThiloteE ThiloteE changed the title We fixed an issue where an exception was thrown for the user after Ct… Fix exception for having inserted an entry and then pressing undo (CTRL + Z) Apr 25, 2023
@ThiloteE ThiloteE changed the title Fix exception for having inserted an entry and then pressing undo (CTRL + Z) Fix undo (CTRL + Z) and redo for inserting an entry Apr 25, 2023
@calixtus
Copy link
Member

Is the CannotUndoException catched?

@Siedlerchr
Copy link
Member

Is the CannotUndoException catched?

Not yet. Was coming from the Undo Manager itself

@Siedlerchr
Copy link
Member

Fixed the occuring of exception by checking for canRedo/canUndo

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 28, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-issue-9737
50b18f7 
* upstream/main: (88 commits)
  Minimal config for openRewrite
  Fix missing #
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  Dissolved FileTab and moved contents to EntryTab
  Renamed CustomEditorFieldsTab to EntryEditorTabsTab
  Fixed antipattern, fixed radiobutton with checkbox
  Renamed ImportExportPreferences to ExportPreferences
  Improve search history by attaching change listener (JabRef#9794)
  Separated WebSearchPrefs and ExportPrefs
  Separated WebSearchTab and ExportTab
  Renamed ImportExportTab to WebSearchTab
  Fix split multiline localization (JabRef#9814)
  New Crowdin updates (JabRef#9834)
  change versin to 0.8.10
  remove  jacoco version config
  Bump org.junit.platform:junit-platform-launcher from 1.9.2 to 1.9.3
  Bump org.jsoup:jsoup from 1.15.4 to 1.16.1
  ...
@Siedlerchr
move exception handling upwards
69613d4
@Siedlerchr
cleanup
ec9f899
@Siedlerchr Siedlerchr merged commit 0f84cdb into JabRef:main May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koppor koppor koppor approved these changes

Assignees
No one assigned
Labels
entry-editor status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

undo keyboard shortcut Ctrl-Z disfunction
5 participants
@brunaoo @koppor @calixtus @Siedlerchr @ThiloteE