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 sorting of all groups and subgroups, recursively #2666

Merged
merged 8 commits into from
Apr 7, 2017
Merged

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Mar 18, 2017

Implements some features from #2599
Recursively sort all groups or selected groups

Replaced Screenshot

grafik

Sometimes the sorting is not always directly correct visible after sorting, This is somehow an update problem of the gui I think. When you switch db back and forth it's correctly there.

- [ ] Change in CHANGELOG.md described
- [ ] Tests created for changes

  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
    - [ ] Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 18, 2017
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.

The code looks good. I would, however, argue that "sort subgroups" should suffice - if you want to sort the whole tree then just right click the root. And maybe add "recursively" to the menu text to make it a bit clearer.

By the way, we should discuss at some point which other menu entries make sense and which we should remove instead of reimplementing them (e.g. I don't see a point in the move actions).

@FXML private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> disclosureNodeColumn;
@FXML private CustomTextField searchField;
@FXML
private TreeTableView<GroupNodeViewModel> groupTree;
Copy link
Member

Choose a reason for hiding this comment

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

Please leave the formatting as it is right now. It is kind of a JavaFX-thing that normally the @FXML tag is on the same line (and I like it that way because otherwise the private fields declaration takes more space)

@Siedlerchr
Copy link
Member Author

Siedlerchr commented Mar 19, 2017 via email

@tobiasdiez tobiasdiez removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 19, 2017
* upstream/master:
  Localization: General: French: Translation of new entries
  Localization: Menu: French: Translation of an entry (#2685)
  Fix #2680 and fix #2667: Swing errors are catched properly and without freezing (#2681)
  Do not log AND throw
  Replace misleading error message for fetcher connection error
  Document CrossRef test
  Fix subtitle detection for CrossRef fetcher
  Revert "Invoke LogMessages.add in JavaFX thread"
  Use global user agent
  Update mockito from 2.7.17 to 2.7.18
  Move GuiAppender to GUI package
  Invoke LogMessages.add in JavaFX thread
  [WIP] Put the PDFAnnotationImporter under Test, enhance FileAnnotationTab (#2640)
  Fix for "Paying off technical debt: almost all utility classes have a private constructor now." (#2672)
  Revert "Paying off technical debt: almost all utility classes have a private constructor now. (#2649)" (#2670)
  Paying off technical debt: almost all utility classes have a private constructor now. (#2649)
  Changed codeformatting for better fxml annotation (#2668)
  Disalbe Google Scholar tests on all CI environments (#2654)
  Fix JSONException in Crossref fetcher as mentioned in #2442
@Siedlerchr
Copy link
Member Author

@tobiasdiez Changed localization, but I dunno how to display the menu entry only when the root nod eis selected?
I read about adding a change listener and then setting the contextmenu. However, I am not sure how that plays together with the binding stuff

@tobiasdiez
Copy link
Member

In createContextMenuForGroup you can use isRoot of the GroupNodeViewModel group to determine if the right-clicked group is the root or not...and then create the menu depending on this value.

However, I would completely leave of the sort subgroups option.

@Siedlerchr
Copy link
Member Author

I thin we should stuck with simply having the sort groups recursively. Then we could omit the explicit chek for root node

* upstream/master: (35 commits)
  Update antlr from 4.6 to 4.7
  Fix build
  fix ID consideration in DuplicateCheck
  Add ArXiv identifier batch lookup (#2710)
  Update mockito from 2.7.19 to 2.7.21
  More defensive identifier list #2708
  Revert "Add more identifier field names #2708"
  Add more identifier field names #2708
  Consider entries as equal if their DOI matches #2708
  Imports
  Imports
  Move duplicate detection to logic
  Reuse edit distance class
  Refactoring
  EntryTypeDialog Fetching Autogenerates BibTeX Key (#2709)
  Add changelog entry
  Increase permitted size of StringUtil
  Make sure that JavaFx shuts down in case another JabRef instance is already open
  Remove obsolete localization strings
  Hide context menu before group edit/add (probably a JavaFX vs Swing problem)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/groups/GroupTreeController.java
#	src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
* upstream/master:
  Implement #1359: collect telemetry (#2283)
  Write groups under tag `group` instead of `groupstree` to enhance compatibility with previous versions (#2704)
  Avoid conversion of single underscores (#2711)
  Fix #628: implement hierarchical keywords (#2703)
@Siedlerchr Siedlerchr merged commit c2fe388 into master Apr 7, 2017
@Siedlerchr Siedlerchr deleted the groupSorting branch April 7, 2017 15:08
Siedlerchr added a commit that referenced this pull request Apr 13, 2017
* upstream/master: (39 commits)
  Fix fetcher test
  Allow failures for fetcher test (#2730)
  Use JabRefExecutor service
  Move DOI fetching to separate thread #2682
  Remove gui dependency in logic (#2726)
  Fixed freeze on Mac OS X when creating/editing groups (#2727)
  Only ask once if telemetry data should be collected
  Update wiremock from 2.5.1 to 2.6.0
  Update mockito-core from 2.7.21 to 2.7.22
  Update log4j to latest version
  Azure test (#2724)
  Fix build
  Move expand filename to FileUtil
  Unicode conversion bibtexkey (#2720)
  Add sorting of all groups and subgroups, recursively (#2666)
  Only check capitalization of note and howpublished fields if they start with a word character
  Remove overhauled @author tag
  Implement #1359: collect telemetry (#2283)
  Add licenses of new dependencies
  Fix cssStyleHelper warnings
  ...
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.

2 participants