JournalAbbreviation search feature#7804
Conversation
Implemented a string filter for the journal abbreviations tab. Not working. Performing a search currently cleares a list of filtered abbreviations and fills it back up with abbreviations that contain the search string. Right now the clearing not only cleares the filtered list but also the list of all abbreviations and I don't unterstand why. Issue JabRef#7751
Changed implementation to use JavaFX's FilteredList.
I think the conversion of the search term to lower-case makes more sense in the contains (now containsLowerCase) method. We already need to convert the terms to compare there, so people expecting contains() to be case-dependent (as it usually is) would be fooled.
|
The documentation includes a screenshot of the abbreviations tab. I could update that, but since the feature is not released yet, it's misleading. |
Siedlerchr
left a comment
There was a problem hiding this comment.
Some minor code imprvements, otherwise lgtm
I do not know about this, but an alternative could be to alter the documentation while mentioning this is valid for v 5.3 only. |
|
For the documentation create a PR and we merge it afterwards, but with gitbook I'm unsure if this doesn't break all... |
k3KAW8Pnf7mkmdSMPHz27
left a comment
There was a problem hiding this comment.
Pretty much everything from my side is nitpicking. View them as suggestions and include them if you agree.
However, I would like you to change JournalAbbreviationsTab.selectNewAbbreviation so that it uses the length of the filtered list instead (perhaps journalAbbreviationsTable.getItems().size() is better).
I meant GitHub doesn't allow me to comment on code that is that far out of the region of your changes 😝 Since |
Ah, I did not see that. But what if the new abbreviation does not match the search criteria? Then setting the selection to the last row in the search result is not very useful. Would it maybe make sense to clear the search before adding new abbreviations? |
Huh, I hadn't thought of that 🤔 |
|
What about adding a label e..g No search results found? |
|
@Siedlerchr the use case is,
|
|
Ah, now I understand. I would simply not select anything after adding, the same as when you search for something and add a new entry to the table. The new entry is not shown because you have filtered the list and as far as I remember it's also not selected |
…into feature/abbreviationsSearch
Are you talking about the main bib-table? It's a little different there as when you add an entry, the entryeditor is opened so there is some visual feedback that an entry was added and users are able to edit them. |
|
+1 for clearing the search when adding a new abbreviation. |
|
Thanks for integrating the flash. This should make it now very obvious that the search has been cleared. UI looks very good to me. I will take a closer look at the source tonight. |
Also added UnitTest for new ColorUtil.toRGBAcode.
k3KAW8Pnf7mkmdSMPHz27
left a comment
There was a problem hiding this comment.
Looks good to me! Change the SimpleStringProperty and it should be good to go.
There are still some issues with which Abbreviation gets selected and how, but I'd consider it unrelated to this PR. Perhaps a follow-up issue? Some of it might be beginner friendly.
One example of this is
- Create a new Abbreviation
- The first column (with default text
Nameshould be selected) - Click on an empty spot of the table
- Press enter and get a
Cannot invoke "javafx.scene.control.TableColumnBase.isEditable()" because the return value of "javafx.scene.control.TablePositionBase.getTableColumn()" is nullmessage (most likely we are selecting the full row rather than the cell, I haven't verified this)
Another seem to be that there always is an empty row in a custom abbreviations list.
Selecting the right column (add2d44) does not completely fix this, but gets rid of the exception. Hitting enter keeps the text-field in the name column selected, but does not commit it. One has to click the text again and hit enter to apply the changes. |
|
Speaking of the selection, I don't know how safe it is to do the selection right after starting the animation for the search-invalidation. Maybe it would be better to do all the actions for the new abbreviation (addAbbreviation, selectNewAbbreviation, editAbbreviation) only after the search is invalidated? Just to make sure the abbreviation is visible when doing the selection? Edit: Tried it out, behaviour is still the same but I think it is safer like this. 4cf14b7 |
|
I would have expected to have the search field above the table, what do the others think? |
|
As the line above the list is already crowded with various controls, I would vote to merge this PR as it is now and redesign the whole dialog in another PR. |
|
Thanks! |
* upstream/main: [Bot] Update CSL styles (JabRef#7824) Pin version of citeproc-java (JabRef#7827) JournalAbbreviation search feature (JabRef#7804) Bump fontbox from 2.0.23 to 2.0.24 (JabRef#7815) Revert "Bump org.eclipse.jgit (JabRef#7820)" (JabRef#7823) Bump bcprov-jdk15on from 1.68 to 1.69 (JabRef#7816) Bump postgresql from 42.2.20 to 42.2.21 (JabRef#7817) Bump org.eclipse.jgit (JabRef#7820) Bump mockito-core from 3.11.0 to 3.11.1 (JabRef#7819) Bump pdfbox from 2.0.23 to 2.0.24 (JabRef#7818) Bump byte-buddy-parent from 1.11.1 to 1.11.2 (JabRef#7821) Bump xmpbox from 2.0.23 to 2.0.24 (JabRef#7822)

I Implemented a string filter for the journal abbreviations tab.
This is not working as of yet. Performing a search currently cleares a list of filtered
abbreviations and fills it back up with abbreviations that contain the
search string. Right now the clearing not only cleares the filtered list
but also the list of all abbreviations and I don't unterstand why.
Fixes #7751
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)