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

Fix setting of title (and simplify BasePanel to LibraryTab) #6129

Merged
merged 25 commits into from
Oct 20, 2020
Merged

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 15, 2020

Fixes koppor#394
Fixes koppor#425

The code is not optimized for many opened tabs. The old code wasn't in too many places. We rely on title calculation of the current tab. It think, this is OK as the program title should reflect the tab title.

Current solution

grafik

Solution of 4b0b662 (OUTDATED)

grafik

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for bigger UI changes)
  • Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

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.

Looks good to me, modulo some smaller suggestions.

@@ -539,11 +507,12 @@ public void updateEntryEditorIfShowing() {
}
}

/**
* Put an asterisk behind the filename to indicate the database has changed.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please investigate if this method is still really necessary. In principle, it should be enough to add listeners to the state manager to be notified about db changes. This would also fix the strange flow of information (from the base panel to the main frame).

Copy link
Member

Choose a reason for hiding this comment

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

I intensivly investigated this issue. I believe this method or something like it is still necessary until we can bind some property to an undomanager, which can provide a dirty flag for each library file.

src/main/java/org/jabref/gui/JabRefFrame.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/JabRefFrame.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/JabRefFrame.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/JabRefFrame.java Outdated Show resolved Hide resolved
@JabRef JabRef deleted a comment from codecov bot Mar 16, 2020
@koppor koppor changed the title Fix setting of title (and remove obsolete comments) [WIP] Fix setting of title (and remove obsolete comments) Mar 21, 2020
@tobiasdiez tobiasdiez marked this pull request as draft April 17, 2020 16:00
@koppor koppor changed the title [WIP] Fix setting of title (and remove obsolete comments) Fix setting of title (and remove obsolete comments) Apr 23, 2020
@koppor koppor self-assigned this Apr 23, 2020
@koppor
Copy link
Member Author

koppor commented Jul 27, 2020

Some review comments cause much work on my side. Since I really like to have my two UI issues fixed, I intend to continue to work on this PR.

Note that we removed an easy indication whether the database is bibtex or biblatex. Since JabRef at some places communicates the mode to the user (e.g., different cleanup dialogs), I would spend time to think about the communication of that.

@tobiasdiez
Copy link
Member

Note that we removed an easy indication whether the database is bibtex or biblatex. Since JabRef at some places communicates the mode to the user (e.g., different cleanup dialogs), I would spend time to think about the communication of that.

What about putting it in the tooltip? Something like

Library has changed. (depending on state)

Mode: BibLaTex
Path: C://....

@Siedlerchr
Copy link
Member

How is the status here? Ready for merge?

@koppor
Copy link
Member Author

koppor commented Sep 2, 2020

The discussion regarding * refs #4161.

@koppor koppor assigned calixtus and unassigned koppor Oct 13, 2020
@calixtus
Copy link
Member

I see some strange behaviour I don't understand and I'd like to ask for some help:
If I bind my list
EasyBind.bindContent(stateManager.getOpenTabs(), EasyBind.map(tabbedPane.getTabs(), tab -> ((LibraryTab) tab).getBibDatabaseContext)) the stateManager var openTabs stays empty. But if I bind EasyBind.bindContent(stateManager.getOpenTabs(), tabbedPane.getTabs()); it works. What am I doing wrong @tobiasdiez ?

@tobiasdiez
Copy link
Member

It's probably again the problem that the listeners are only weak-references, so the mapped list is garbage-collected. You should be able to resolve this by keeping a class-reference of EasyBind.map(tabbedPane.getTabs(), tab -> ((LibraryTab) tab).getBibDatabaseContext), i.e. introduce a variable for it, and then use bindContent on this variable.

@calixtus
Copy link
Member

You are right, that solved the issue. Thanks!

@calixtus calixtus changed the title Fix setting of title (and remove obsolete comments) Fix setting of title (and simplify BasePanel to LibraryTab) Oct 18, 2020
@calixtus
Copy link
Member

My solution is now this:

modification-asterisk filename – path-fragment

The modification-asterisk (*) is shown if the file was modified since last save
(path-fragment is only shown if filename is not (globally) unique)

Example:
*jabref-authors.bib – testbib

The window title is bound to the tab title (just like notepad++), the bibtex mode is only displayed in the tooltip.
Comments? Suggestions? Damnations?

@calixtus calixtus marked this pull request as ready for review October 19, 2020 20:52
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui labels Oct 19, 2020
@calixtus
Copy link
Member

grafik

@calixtus
Copy link
Member

calixtus commented Oct 19, 2020

For your information: This PR escalated mainly because of this small comment.
#6129 (comment) 😉

@tobiasdiez
Copy link
Member

Pffff... in der Beschränkung zeigt sich der Meister 😻

Not sure when I'll be able to review this PR, but after scrolling through it I really like it.

@Siedlerchr
Copy link
Member

Wow! Huge work ;) I also think it's good
RIP variable BasePanel 😭

@koppor koppor merged commit 4cf2fb0 into master Oct 20, 2020
@koppor koppor deleted the fix-title branch October 20, 2020 21:27
koppor pushed a commit that referenced this pull request Jul 1, 2023
80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581)
b6fb00e415 Create arachnologische-mitteilungen.csl (#6375)
2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580)
fd230a7073 Create veterinary-clinical-pathology.csl (#6372)
ac0afa3cae Create dedicated Basque APA file (#6370)
fee0677a88 Update chicago-author-date.csl (#6289)
ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230)
f4116db325 Create Turcica.csl (#6240)
0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211)
e73bf46674 Update vancouver.csl (#6156)
b73c3ef193 Create independent TF AIP Style
befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181)
048c9bddbc Add csl for SDMI (#6129)
1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567)
b77084255f Update the-quarterly-journal-of-economics.csl (#6572)
cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577)
9e9e08c219 Update isara-iso-690.csl (#6578)
4b4e8f442d Create silva-fennica.csl (#6568)
dd8760bb2b Update american-journal-of-botany.csl (#6569)
8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560)
016050c4b7 Update geographie-et-cultures.csl (#6563)
e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa
calixtus pushed a commit that referenced this pull request Jul 1, 2023
…ce (#10048)

80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581)
b6fb00e415 Create arachnologische-mitteilungen.csl (#6375)
2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580)
fd230a7073 Create veterinary-clinical-pathology.csl (#6372)
ac0afa3cae Create dedicated Basque APA file (#6370)
fee0677a88 Update chicago-author-date.csl (#6289)
ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230)
f4116db325 Create Turcica.csl (#6240)
0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211)
e73bf46674 Update vancouver.csl (#6156)
b73c3ef193 Create independent TF AIP Style
befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181)
048c9bddbc Add csl for SDMI (#6129)
1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567)
b77084255f Update the-quarterly-journal-of-economics.csl (#6572)
cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577)
9e9e08c219 Update isara-iso-690.csl (#6578)
4b4e8f442d Create silva-fennica.csl (#6568)
dd8760bb2b Update american-journal-of-botany.csl (#6569)
8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560)
016050c4b7 Update geographie-et-cultures.csl (#6563)
e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa

Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>
koppor pushed a commit that referenced this pull request Jul 15, 2023
a97dbb32c5 Update studii-teologice.csl (#6591)
e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594)
c8abbcdd88 Update acm-sig-proceedings.csl (#6595)
725ace4a40 Create uppsala-university-library-harvard.csl (#6574)
a973041e0e update bluebook-law-review.csl (#6583)
0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589)
80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581)
b6fb00e415 Create arachnologische-mitteilungen.csl (#6375)
2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580)
fd230a7073 Create veterinary-clinical-pathology.csl (#6372)
ac0afa3cae Create dedicated Basque APA file (#6370)
fee0677a88 Update chicago-author-date.csl (#6289)
ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230)
f4116db325 Create Turcica.csl (#6240)
0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211)
e73bf46674 Update vancouver.csl (#6156)
b73c3ef193 Create independent TF AIP Style
befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181)
048c9bddbc Add csl for SDMI (#6129)
1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567)
b77084255f Update the-quarterly-journal-of-economics.csl (#6572)
cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577)
9e9e08c219 Update isara-iso-690.csl (#6578)
4b4e8f442d Create silva-fennica.csl (#6568)
dd8760bb2b Update american-journal-of-botany.csl (#6569)
8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560)
016050c4b7 Update geographie-et-cultures.csl (#6563)
e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: a97dbb32c57c8c00e47422dae4ba4f480e753da5
koppor pushed a commit that referenced this pull request Aug 1, 2023
0749a19b83 Update journal-of-experimental-botany.csl (#6604)
b1768724fe Update modern-language-association.csl (#6606)
dd364c1815 Create modern-language-association-for-JEI.csl (#6593)
6cb436997b Partial update of APA styles for 1.0.2, including software, legal, most localizations (#6270)
d7c4ebec85 fix film/video authors for american-sociological-association.csl (#6602)
a97dbb32c5 Update studii-teologice.csl (#6591)
e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594)
c8abbcdd88 Update acm-sig-proceedings.csl (#6595)
725ace4a40 Create uppsala-university-library-harvard.csl (#6574)
a973041e0e update bluebook-law-review.csl (#6583)
0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589)
80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581)
b6fb00e415 Create arachnologische-mitteilungen.csl (#6375)
2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580)
fd230a7073 Create veterinary-clinical-pathology.csl (#6372)
ac0afa3cae Create dedicated Basque APA file (#6370)
fee0677a88 Update chicago-author-date.csl (#6289)
ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230)
f4116db325 Create Turcica.csl (#6240)
0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211)
e73bf46674 Update vancouver.csl (#6156)
b73c3ef193 Create independent TF AIP Style
befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181)
048c9bddbc Add csl for SDMI (#6129)
1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567)
b77084255f Update the-quarterly-journal-of-economics.csl (#6572)
cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577)
9e9e08c219 Update isara-iso-690.csl (#6578)
4b4e8f442d Create silva-fennica.csl (#6568)
dd8760bb2b Update american-journal-of-botany.csl (#6569)
8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560)
016050c4b7 Update geographie-et-cultures.csl (#6563)
e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565)

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

Path suffix of title should not be removed Full path of current bib file in title
4 participants