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

Add new menu entry to remove braces from selection, aka unprotect it. #9968

Conversation

BeeJay28
Copy link
Contributor

@BeeJay28 BeeJay28 commented Jun 1, 2023

Fixes #9950

Add an additional right click menu entry to "unprotect text selection", which strips the selected text of any curly-brace pairs.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable). (UnprotectTermsFormatterTest.java does the testing)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request. (No information there)
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository. No documentation for this little right-click menu entry available. If an entry is necessary, then perhaps in general on how terms-protection works, what it is useful for and how to use it.

UI Changes:

Unprotect with selected text
Unprotect with UNselected text

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label Jun 1, 2023
BeeJay28 and others added 3 commits June 4, 2023 11:28
…-menu-issue-9950' of github.com:BeeJay28/jabref into add-remove-protect-in-entire-text-option-in-right-click-menu-issue-9950
@BeeJay28 BeeJay28 marked this pull request as ready for review June 4, 2023 11:52
Siedlerchr
Siedlerchr previously approved these changes Jun 5, 2023
@Siedlerchr Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: changes required Pull requests that are not yet complete labels Jun 5, 2023
@koppor
Copy link
Member

koppor commented Jun 6, 2023

The PR misses a screenshot:

image

It works there, but does not work if nothing is selected:

image

Three options:

a) gray out option if nothing is selected
b) Unprotect ALL if clicked
c) change text to "Unprotect" if nothing is selected and unprotect everything when clicked

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

It does not behave properly when nothing is selected. See comments.

Minor: I had to fix a CHANGELOG merge error.

@koppor koppor added the ui label Jun 6, 2023
@BeeJay28
Copy link
Contributor Author

BeeJay28 commented Jun 6, 2023

I was unsure of what it should exactly do if nothing is selected, so I just left it at the easiest option. Thanks for the feedback, I'll get on it.
And thank you for the screenshot. Tbh, I just had issues screenshotting while having the right click menu up (it would automatically close before capture). Thank you for that too 😅

@koppor
Copy link
Member

koppor commented Jun 6, 2023

And thank you for the screenshot. Tbh, I just had issues screenshotting while having the right click menu up (it would automatically close before capture). Thank you for that too 😅

I am using Greenshot on Windows with Hotkey Ctrl+F1 and that works most of the time. I also have experiences using GIMP, where one can say: Take a screenshot in 10 seconds. Then, one only needs "patience".

Thank you for continuing working on this!

@BeeJay28
Copy link
Contributor Author

BeeJay28 commented Jun 6, 2023

Three options:

a) gray out option if nothing is selected b) Unprotect ALL if clicked c) change text to "Unprotect" if nothing is selected and unprotect everything when clicked

I picked option a)

@BeeJay28 BeeJay28 requested a review from koppor June 6, 2023 14:25
@koppor
Copy link
Member

koppor commented Jun 6, 2023

@ThiloteE OK for me that the option is disabled; consistent to other UI. I would have found the changing label better, but if it's OK for you to move forward, it is also for me :)

@Siedlerchr Siedlerchr merged commit e9e1d3f into JabRef:main Jun 6, 2023
Siedlerchr added a commit that referenced this pull request Jan 1, 2024
* upstream/main: (68 commits)
  Fix issue 9863 - Change Tab selection order (#9907)
  New Crowdin updates (#9994)
  Enable editing typo with double clicking on field name in custom entry type (#9977)
  Remove "Field name:" from localization - we already have "Field name" (#9991)
  Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989)
  Use environment variables for hostname detection (#9910)
  Add cleanup activity for URL field (#9970)
  Fix freezing when fetching IBSN and no results are found (#9987)
  Make Group(Node)TreeViewModel more OO (#9978)
  Fix container for group item count still visible if display count is off  (#9980)
  Fix paste of BibTeX data (#9985)
  Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981)
  Add new menu entry to remove braces from selection, aka unprotect it. (#9968)
  User specific comment (#9727)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975)
  Fix typos
  Remove non-needed link
  Fix typos
  Enable RemoveJavaDocAuthorTag (#9973)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unprotect selection
4 participants