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

Redesign "Manage field names & content" dialog #8892

Merged
merged 48 commits into from
Jun 22, 2022
Merged

Redesign "Manage field names & content" dialog #8892

merged 48 commits into from
Jun 22, 2022

Conversation

HoussemNasri
Copy link
Member

@HoussemNasri HoussemNasri commented Jun 7, 2022

Fixes #6536

Question: Looking at other dialogs in JabRef, I noticed a pattern where actions are always at the bottom of the dialog, but I can't seem to reformulate UI logic to follow that pattern. I think one solution could be to divide the UI across 2 dialogs. One for 'Edit field value of selected references' and another for 'Copy or move the value of one field to another'. What are your thoughts on this?

Automatic Field Editor

Overview

This PR aims to update the appearance & logic of the 'Manage field names & content' dialog. Although the problem seems trivial to solve, I had to make many decisions during the design phase, hence why I included user stories and acceptance criteria's.

User Stories

📗 Summary

As a user of JabRef

I want to clear one field in a collection of bib entries automatically


📗 Summary

As a user of JabRef

I want to change the value of one field in a collection of bib entries automatically

📋 Acceptance Criteria

Background

Given 'Manage field names & content' dialog is shown

Acceptance Criteria 1

When user enter an illegal value for the given field
Then show an error message

Acceptance Criteria 2

When user enter a legal value to be set for the specified field

And user clicks 'set field'

And 'Overwrite existing field values' checkbox is NOT selected

Then set the entered value to entries with the specified field empty

Acceptance Criteria 3

When user enter a legal value to be set for the specified field

And user clicks 'set field'

And 'Overwrite existing field values' checkbox is selected

Then set the entered value to all entries even if the specified field is not empty


📗 Summary

As a user of JabRef

I want to copy the value of one field to another field in a collection of bib entries automatically

📋 Acceptance Criteria

Background

Given 'Manage field names & content' dialog is shown

Acceptance Criteria 1

When the source and destination fields are not compatible (they don't hold the same type of data)
Then show an error message

Acceptance Criteria 2

When user attempt to copy the values of source field to destination field

And 'Overwrite Non empty field' is NOT selected

Then only update the destination field if it's empty


📗 Summary

As a user of JabRef

I want to move/cut the value of one field to another field in a collection of bib entries automatically

📋 Acceptance Criteria

Background

Given 'Manage field names & content' dialog is shown

Acceptance Criteria 1

When the source and destination fields are not compatible (they don't hold the same type of data)
Then show an error message

Acceptance Criteria 2

When 'Overwrite Non empty field' is NOT selected

Then disable the 'Move values' button


📗 Summary

As a user of JabRef

I want to append some value to one field in a collection of bib entries automatically

📋 Acceptance Criteria

Background

Given 'Manage field names & content' dialog is shown

Acceptance Criteria 1

When user enter an illegal value for the given field
Then show an error message

Acceptance Criteria 2

When 'Overwrite Non empty field' is NOT selected
Then disable 'Append value' button


📗 Summary

As a user of JabRef

I want to swap the values of two fields in a collection of bib entries automatically

📋 Acceptance Criteria

Background

Given 'Manage field names & content' dialog is shown

Acceptance Criteria 1

When 'Overwrite Non empty field' is NOT selected
Then disable 'swap values' button

Wireframe

Screenshot 2022-06-04 023327

Screenshots

Screenshot 2022-06-11 181620

Screenshot 2022-06-11 181635

Screenshot 2022-06-11 181646

💾 Technical Information

  • We can use org.jabref.logic.integrity.FieldCheckers#getForField() to validate a field value.

.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • 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.
  • 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.

@Siedlerchr
Copy link
Member

Like the idea! I am just missing the option for renaming the field here? e.g.
In the current dialog, I can rename my existing field "title" to a field "customtitle" and in the biblatex source tab you see that title = { mytitle } got replaced with customtitle = { mytitle }.
Is this missing in your UI?

Regarding the Design:
We yesterday talked about the styling of the New Entry dialog and our solution was to come with tabs for example as seen in the library properties. This has a clear separation of tasks, and it also avoids long dialogs with much vertical size. This is especially often a problem for users with a small vertical screen size.

@HoussemNasri
Copy link
Member Author

I am just missing the option for renaming the field here? e.g.
In the current dialog, I can rename my existing field "title" to a field "customtitle" and in the biblatex source tab you see that title = { mytitle } got replaced with customtitle = { mytitle }.

I think I misunderstood the rename button responsibility after reading the code comment below. But, since empty fields aren't stored then it makes perfect sense. So if I understand this correctly then renaming 'Move value' to 'Rename field' should solve it?

* Move contents from one field to another for a Collection of entries.
*
* @param entries The entries to do this operation for.
* @param field The field to move contents from.
* @param newField The field to move contents into.
* @param overwriteValues If true, overwrites any existing values in the new field. If false, makes no change for
* entries with existing value in the new field.
* @return A CompoundEdit for the entire operation.
*/
private static UndoableEdit massRenameField(Collection<BibEntry> entries, Field field, Field newField,

We yesterday talked about the styling of the New Entry dialog and our solution was to come with tabs for example as seen in the library properties.

Thank you; this looks way better than having 2 dialogs or putting everything in one. However, I had a look at 'Library properties' and it has no action buttons on the body. The only action button is 'Apply.'. Maybe we can update the dialog's buttons on tab switch?

@Siedlerchr
Copy link
Member

I think we need to just distinguish the semantics here, although it's technically the same process:

  1. Moving a value from A to B. e.g. the convert to biblatex moves the content of the field journal to journaltitle
  2. Rename a field, for example mendeley-tags to mendeley_tags

Both are actually the same technically. Maybe just another button Rename Fields
Regarding the action button: You can just make it a "close button". See for example in Find unlinked files.

@HoussemNasri
Copy link
Member Author

HoussemNasri commented Jun 10, 2022

I created two action buttons: 'Keep Modifications' and 'Revert'. Clicking 'Keep Modifications' will save the modifications to UndoManager as a CompoundEdit (Thus users can undo them by clicking the undo arrow from the global bar), while clicking 'Revert' will undo all changes (Like you have never opened the dialog).

@HoussemNasri HoussemNasri marked this pull request as ready for review June 11, 2022 15:23
@HoussemNasri
Copy link
Member Author

This PR has already grown larger than I expected. Some of the requirements, such as validating entered field values against selected field using FieldCheckers, will be left to another PR. I'd also like to write some test cases for the view models.

@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 15, 2022
@calixtus calixtus requested a review from Siedlerchr June 15, 2022 18:15
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Lgtm in general, just some suggestion

@calixtus calixtus merged commit 9701e35 into JabRef:main Jun 22, 2022
@calixtus
Copy link
Member

Thanks! Great addition to JabRef!

@HoussemNasri HoussemNasri deleted the automatic-field-editor branch June 22, 2022 19:27
@ThiloteE
Copy link
Member

ThiloteE commented Jun 22, 2022

Hey, just tried this. Overall I like it! I think I already found a bug though T.T

How to reproduce:

  1. Add two entries
@Article{x1,
  a = {1},
  b = {2},
}
@Article{x2,
  b = {3},
}
  1. Select both entries
  2. Go to two fields tab
  3. From a, To b; Enable Overwrite non empty fields; Press copy value

Result:

@Article{x1,
  a = {1},
  b = {1},
}
@Article{x2,
}

Expected:

@Article{x1,
  a = {1},
  b = {1},
}
@Article{x2,
  b = {3},
}

@HoussemNasri
Copy link
Member Author

@ThiloteE Nice catch! I kind of expected it to be buggy at this stage. I'll be making another follow up PR to address this issue and other issues that may come up. I will also include test cases to help catch bugs faster.

Siedlerchr added a commit that referenced this pull request Jun 23, 2022
* upstream/main: (27 commits)
  Add gitpod config (#8921)
  Fix javafx version not displayed in About Dialog (#8918)
  Redesign "Manage field names & content" dialog (#8892)
  Rework IACR fetcher (#8904)
  Bump h2-mvstore from 2.1.212 to 2.1.214 in /buildSrc (#8915)
  Bump unoloader from 7.3.3 to 7.3.4 (#8912)
  Bump libreoffice from 7.3.3 to 7.3.4 (#8913)
  Bump tika-core from 2.4.0 to 2.4.1 (#8914)
  Bump org.eclipse.jgit from 6.1.0.202203080745-r to 6.2.0.202206071550-r (#8916)
  Squashed 'buildres/csl/csl-styles/' changes from e740261..845dee0 (#8903)
  Bump flowless from 0.6.9 to 0.6.10 (#8898)
  Bump postgresql from 42.3.5 to 42.4.0 (#8900)
  Bump mockito-core from 4.6.0 to 4.6.1 (#8899)
  Bump antlr-runtime from 3.5.2 to 3.5.3 (#8897)
  Update to MADR 3.0.0-beta.2 (#8896)
  Increase the network connection timeout and improve error message (#8894)
  Fix linux terminal opening process error (#8891)
  Fix exception if linked file has masked umlauts / invalid characters in path (#8851)
  Remove obsolte CHANGELOG.md entry
  Revert "Fix right clicking a group and choosing "remove selected entries from this group" leads to error when Bibtex source tab is selected (#8821)"
  ...

# Conflicts:
#	build.gradle
Siedlerchr added a commit to prashant1712/jabref that referenced this pull request Jun 27, 2022
* upstream/main:
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
  Pass the data as a string (JabRef#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
  Add gitpod config (JabRef#8921)
  Fix javafx version not displayed in About Dialog (JabRef#8918)
  Redesign "Manage field names & content" dialog (JabRef#8892)
  Rework IACR fetcher (JabRef#8904)
  Bump h2-mvstore from 2.1.212 to 2.1.214 in /buildSrc (JabRef#8915)
  Bump unoloader from 7.3.3 to 7.3.4 (JabRef#8912)
  Bump libreoffice from 7.3.3 to 7.3.4 (JabRef#8913)
  Bump tika-core from 2.4.0 to 2.4.1 (JabRef#8914)
  Bump org.eclipse.jgit from 6.1.0.202203080745-r to 6.2.0.202206071550-r (JabRef#8916)
Siedlerchr added a commit to LIM0000/jabref that referenced this pull request Jun 28, 2022
* upstream/main:
  Fix for Dark theme not readable (JabRef#8929)
  Find Unlinked files should ignore Thumbs.db, etc (JabRef#8800)
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
  Pass the data as a string (JabRef#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
  Add gitpod config (JabRef#8921)
  Fix javafx version not displayed in About Dialog (JabRef#8918)
  Redesign "Manage field names & content" dialog (JabRef#8892)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automatic-field-editor 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.

"Manage field names & content" logic and appearance
4 participants