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 welcome screen to JabRef GUI that displays when no database/library is open #12017

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

HaynesCrossley-ANU
Copy link

Closes https://github.com/koppor/jabref/issues/96 by implementing a 'welcome' screen when no database/library is open in Jabref.

Added:

  • WelcomeScreen.java - representing the welcome screen elements which includes links to open a new library or an existing library (utilizing existing logic to handle these actions).

Modified:

  • JabRefFrame.java - added some logic that checks if a database is open, and then toggles the visibility of the welcome screen accordingly.

Demonstration:

java_gRmosu6sdd

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.


// Title
Label welcomeLabel = new Label("Welcome to JabRef!");
welcomeLabel.setFont(new Font("Arial", 40));
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 add all the font and color stuff to the Base.css file?
It should also look nice with dark theme and customizable by the user

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I have moved the styling to Base.css. See screenshots below for light and dark modes:

image

image

Let me know if anything further needs to be changed.

5d50f08

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Overall looks already good
please extract font size and color stuff etc to css

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label Oct 18, 2024
Comment on lines 270 to 274
// Ensure divider position is set before rendering
splitPane.setDividerPositions(0.2);

// Ensure smooth transition by updating the layout immediately
Platform.runLater(() -> splitPane.setDividerPositions(0.2));
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand that. Both values are equal. I think, only the statement on line 271 is required?

The code of org.jabref.gui.frame.JabRefFrame#updateDividerPosition shows different values. I think you should just call

splitPane.setDividerPositions(preferences.getGuiPreferences().getSidePaneWidth() / splitPane.getWidth());

Copy link
Author

Choose a reason for hiding this comment

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

You are right - splitPane.setDividerPositions(preferences.getGuiPreferences().getSidePaneWidth() / splitPane.getWidth()); works fine!

eab1844

updateSidePane();

// Set the preferred width and divider position explicitly
sidePane.setPrefWidth(260);
Copy link
Member

Choose a reason for hiding this comment

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

260 is a new value. Should be stored as constant in this class. - Why is sidePane used and not SplitPane?


// Force divider to a known position (e.g., 0.2 of the total width)
LOGGER.info("Forcing divider position after adding tab.");
splitPane.setDividerPositions(0.2);
Copy link
Member

Choose a reason for hiding this comment

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

See above.

setSpacing(10);

// Title
Label welcomeLabel = new Label("Welcome to JabRef!");
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 not translated. You need to use Localization.lang. See https://devdocs.jabref.org/code-howtos/localization.html#localization for details.

@InAnYan How did you do it in the AiTab with links and sentences? Did we come up with a good solution.

@HaynesCrossley-ANU I am asking for help here, because for translation, we aim for translating whole sentences. Reason: In different languages, the ordering of words within a sentence is different. (Because the grammar differs from langauge to langauge)

TextFlow openLibraryFlow = new TextFlow(orExistingDatabaseText, openLibraryLink);
openLibraryFlow.setTextAlignment(TextAlignment.CENTER);

// Add elements to the VBox
Copy link
Member

Choose a reason for hiding this comment

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

Do not add comments just stating the words of the statement. You can remove these kinds of comments.

Copy link
Author

Choose a reason for hiding this comment

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

Noted thank you - comments removed.

e15cdb9

@chethin-ANU
Copy link

Hey @koppor

I am in the same team as @HaynesCrossley-ANU and @surymishra. I will be contributing to this PR as well. I'm wondering whether there is a preferred way to translate sentences that have links? There are two simple solutions.

  1. Make the entire sentence a link. In my opinion this would look bad.
  2. Alternatively we can make the statements shorter. Rather than having "open an existing library" we could have "existing library". Maybe convert the links into buttons?

If neither approach is suitable, I'm wondering what techniques have been used in the past for this project?

@koppor koppor removed the status: changes required Pull requests that are not yet complete label Oct 23, 2024
@InAnYan
Copy link
Collaborator

InAnYan commented Oct 23, 2024

@ch, hi! You can do this:

Replace link with %0, and then after you call Localization.lang replace %0 with link.

You can actually use whole sentence as a link, or use a TextFlow with Text and Hyperlink

@chethin-ANU
Copy link

@InAnYan, thank you for the response. I will apply that and update the PR once that is sorted out

@chethin-ANU
Copy link

Hey @koppor,

All requested changes have been made. We would like a review and feedback if any other changes are necessary.

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

All requested changes have been made. We would like a review and feedback if any other changes are necessary.

No. Please do "Replace link with %0, and then after you call Localization.lang replace %0 with link."

Good:

image

Bad:

image

Comment on lines 22 to 24
/**
* A simple Welcome Page for the JabRef application.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Remove JavaDoc - the class name already says this.

Choose a reason for hiding this comment

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

Noted thanks! comments removed

@chethin-ANU
Copy link

@koppor

I'm having trouble understanding the implementation of "after you call Localization.lang replace %0 with link". For instance doing Localization.lang("Open a %0", link) will return a correctly translated string. How do I then divide that between a Text and Hyperlink? Adding onto the complexity of different grammars, it is possible that the hyperlink may appear either before or after the text.

@koppor koppor mentioned this pull request Oct 26, 2024
7 tasks
@calixtus
Copy link
Member

calixtus commented Oct 26, 2024

Hi, I like your implementation of the WelcomePage, especially with the changes that were suggested. However: I would prefer to have the welcome page either as a placeholder (for some nice implementation ideas see https://stackoverflow.com/questions/35239420/display-label-if-tabpane-has-no-tabs ) or as a separate tab.
That would be opened, if no other tab open and stays open, until the user explicitly closes it, and can maybe reopened with a command in the help menu. An example for that behaviour can bee seen in vs code.
The current approach hides the sidebar if no database is opened, but i think the user would like to have the sidebar for the web fetchers.
Thanks.

@koppor
Copy link
Member

koppor commented Oct 26, 2024

Side comment: We use sentence case. A "new library" is "created", "open" is for existing libraries.

I'm having trouble understanding the implementation of "after you call Localization.lang replace %0 with link". For instance doing Localization.lang("Open a %0", link) will return a correctly translated string. How do I then divide that between a Text and Hyperlink? Adding onto the complexity of different grammars, it is possible that the hyperlink may appear either before or after the text.

Side information - what we meant:

String part1 = Localization.lang("Create a %0\nor %1");
part1 = part1.replace("%0", Localization.lang("new library");
part1 = part1.replace("%1", Localization.lang("open an existing library");

BUT we think, the terms should be the same terms as in the menu.

The menu is as follows:

image

After "Welcome to JabRef", add a heading "Start" and below two actions: "New library" and "Open library". Similar to VS.Code:

image

  • Please also "Welcome to JabRef" without exclamation mark.

@koppor koppor marked this pull request as draft October 26, 2024 20:09
@koppor koppor added the status: changes required Pull requests that are not yet complete label Oct 26, 2024
@koppor
Copy link
Member

koppor commented Dec 9, 2024

Closing this issue due to inactivity 💤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants