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 select all buttons to change dialog #5803

Merged
merged 5 commits into from
Jan 3, 2020
Merged

Add select all buttons to change dialog #5803

merged 5 commits into from
Jan 3, 2020

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Jan 1, 2020

when merging entries
Fixes JabRef#385

grafik

TODO: Hide checkbox when switching between entry modified and added/deleted
@Siedlerchr
Copy link
Member Author

After talk with @koppor
grafik

Use all current changes from JabRef = do nothing = cancel
select all changes from jabref = uncheck all checkboxes and set all radiobuttons in merge to left
select all changes from disk = check all checkboxes and set all radiiobuttons to right

However, I still need to find a proper binding way to hide the checkbox in the merge entries when switching between the merge entries and added/deleted

@koppor
Copy link
Member

koppor commented Jan 2, 2020

I would group the buttons "Select ..." together (on the left), then a small dividing space and then the other two buttons which will lead to something really changing (including a close of the dialog).

@tobiasdiez
Copy link
Member

I find the dialog with 6 new buttons relatively complex. As the user it's not really clear what is meant with "accept" vs "select". Personally, I prefer the dichotomy between accept & dismiss changes. Especially, "changes from JabRef" is somewhat misleading as the only changes are on the disk relative to the currently in-memory version.

Moreover, selection of all changes in the entry diff dialog is perhaps a bit too advanced to be placed that prominently. What about adding the functionality to the left / right arrows at the top?

@koppor
Copy link
Member

koppor commented Jan 2, 2020

2nd point with arrows and selection: +1

Regarding the plenty of buttons. I'm unsure. Maybe, it just helps to move the selection buttons below the list of changes.

I know that a design like the merge dialog in IntelliJ would be better. However, we currently don't have the man power for that...

@Siedlerchr
Copy link
Member Author

grafik

I incorporated the feedback and made now the arrows clickable for select all radio buttons (added tooltip)
and I moved the buttons mark and unmark to the left

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

This looks very nice. Good job!

@Override
public void setAccepted(boolean accepted) {
super.setAccepted(accepted);
if (accepted) {
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer needed now, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, this is still needed. It ensures that when you click on "mark all changes" that the radio buttons on the right are selected and the merge entry will be the correct one.

@Siedlerchr Siedlerchr merged commit f11949f into master Jan 3, 2020
@Siedlerchr Siedlerchr deleted the quickselectall branch January 3, 2020 14:29
Siedlerchr added a commit that referenced this pull request Jan 4, 2020
* upstream/master:
  Add select all buttons to change dialog (#5803)
  Squashed 'src/main/resources/csl-locales/' changes from 9785a6e358..a3e8843f75
  Squashed 'src/main/resources/csl-styles/' changes from 49a1841..b2fbe15
  Fix CSL update
  Add new authors
  Remove ACM Fetcher in one more check
  Adapt WebFetchersTest
  Disable ACM Fetcher
  Try to fix branch name on cleanup pr action
  Remove obsolete class (#5801)
  Try to use right ref
  Run tests only once at pull requets
  Add ignored dependency
  Fix "&" on previews (#5786)
  [WIP] Batch Insert entries (#5691)
koppor pushed a commit that referenced this pull request Jan 15, 2022
5563ccc Update american-society-for-microbiology.csl (#5842)
19ab80f Merge pull request #5843 from POBrien333/patch-1002
d4a6c6d double trouble
424f7fe in-text >> superscript for T&F ACS style
605253c Merge pull request #5837 from POBrien333/patch-998
eb5417d Merge pull request #5838 from alessandro-gentilini/patch-1
7d2d3f5 Fix typo
9f557b5 Re-indent CSL styles
f587e60 polyglot style
3a3fe2c Create journal-fur-kulturpflanzen-journal-of-cultivated-plants.csl
cb24633 Merge pull request #5826 from dstark/patch-7
2590a6c Merge pull request #5833 from POBrien333/patch-997
5b2481e Create nist-techpubs-jres.csl (#5756)
d2a1a49 Re-indent CSL styles
42c51d1 Update tyndale-bulletin.csl
8dae693 rework style
070586b Localize IEEE dates (#5835)
588fbfe Hopefully fix sorting in CSE author-date (#5834)
1291d72 Update physiotherapy-theory-and-practice.csl
1a64076 New Style for "Neue Kriminalpolitik (Deutsch)" (#5586)
eac93a0 Create physiotherapy-theory-and-practice.csl
2e7a9f6 Merge pull request #5832 from POBrien333/patch-996
4dfad5f Update administrative-science-quarterly.csl
f56db0f Merge pull request #5829 from benthamite/patch-1
526b0b3 Update effective-altruism-wiki.csl
e5b11eb Update effective-altruism-wiki.csl
f2a7301 Update effective-altruism-wiki.csl
5b166c2 Update university-of-roehampton-harvard.csl (#5831)
b231514 Update european-society-of-cardiology.csl (#5519)
dbd902c Re-indent CSL styles
fe8892b Update effective-altruism-wiki.csl
734fa7d Update effective-altruism-wiki.csl
2430a32 Create effective-altruism-wiki.csl
a5101b6 update modern-pathology.csl (#5828)
16f77c8 Update tyndale-bulletin.csl
ea8804e Create the-korean-journal-of-mycology.csl (#5822)
22d8be0 Patch 3 (#5825)
0220ba2 Create jcom.csl (#5819)
cd7b72b Create taylor-and-francis-council-of-science-editors-numeric.csl (#5824)
0033383 Create gomis-senegalese-medicine-thesis.csl (#5774)
b221bcc Create mycobiology.csl (#5821)
d7056f9 Re-indent CSL styles
04ae08b Update tyndale-bulletin.csl
24bd577 Re-indent CSL styles
e08d431 clean up logging
f6ac2a6 Update tyndale-bulletin.csl
e4c07f8 this should be the PR repo name
7f9936c fetch pull request from originating repo
f232f51 Update tyndale-bulletin.csl
e2ae4fc Update tyndale-bulletin.csl
f09f8db Update Sheldon to csl-styles 2.0
a4d7dec re-indent style
a903664 sheldon
7db75fc sheldon
f08dc96 sheldon
35b8bfa sheldon
04e28ca pull req checkout
9c9d061 pull req checkout
9249490 sheldon reindent
9792be6 sheldon reindent
3bced06 reindent & commit
7cc9735 Update styles README and GitHub Actions with 1.0.2 information (#5803)
205c4ea remaining dependents
9458056 fix broken styles
07ba7af fix dependents
e2e7842 Fix term hacks for transition to CSL 1.0.2 (#5801)
5c8c5e9 Replace "sub verbo" with "sub-verbo" (#5799)
7ec637c Fix CSL styles
bcd3054 update to csl/citeproc-ruby 2.0
ec13830 bundle update

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 5563ccc
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.

Quick change of default in Merge Entries Dialog
3 participants