-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 checks and warnings in SLR for non-empty directories #11088
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes, otherwise: LGTM
Thank you for diving into the SLR topic. This is a very highly-ranked topic by me.
src/main/java/org/jabref/gui/slr/ManageStudyDefinitionView.java
Outdated
Show resolved
Hide resolved
To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
Example:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See CSS comment
a39552d
to
5132142
Compare
@@ -1308,6 +1308,10 @@ We want to have a look that matches our icons in the tool-bar */ | |||
-fx-text-fill: -jr-warn; | |||
} | |||
|
|||
.warning-label { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why couldn't you use the class .warning-message
(instead of introducing a new class) with the color -jr-warn
? (See line 76)
JabRef tries to have consistent UI colors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, actually I have night light on on my laptop, so couldn't see the orange warning too well. So I stuck to red.
Then when I realized night light was the reason, removed it. Sorry.
Thank you, edited! |
@@ -2639,3 +2639,8 @@ Redownload\ missing\ files=Redownload missing files | |||
Redownload\ missing\ files\ for\ current\ library?=Redownload missing files for current library? | |||
|
|||
File\ directory\ needs\ to\ be\ set\ or\ does\ not\ exist.\nPlease\ save\ your\ library\ and\ check\ the\ preferences.=File directory needs to be set or does not exist.\nPlease save your library and check the preferences. | |||
|
|||
Note\:\ The\ study\ directory\ should\ be\ empty.=Note: The study directory should be empty. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, the UI pattern of JabRef is to use colors to indicate "Note" and "Warning".
OK, we do have some strings with "Warning:" in front. Too much work to discuss this now. I'll just merge and we can do a follow-up for consistent UI later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
Fix koppor#600
Fix koppor#620
Description
When starting a new Systematic Literature Review (SLR), earlier, the users were allowed to create a new SLR (the
Start survey
button wasn't blocked) even if a non-empty directory was selected. There were no notes or warnings as well:This would also sometimes cause the following exception to be thrown:
Through the changes made,
Start survey
button is blocked if a non-empty directory is selected:The
Start survey
button is now only enabled when the selected directory is empty (thus avoiding errors as well):Edit: For purposes of uniformity, we decided to stick to the global
warning-message
inBase.css
instead of the red warning label depicted above:Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)- [ ] Tests created for changes (if applicable)