Skip to content

Conversation

@Shreyasdone
Copy link
Contributor

@Shreyasdone Shreyasdone commented Dec 12, 2025

Closes #14407

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • [/] I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
  • I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Dec 12, 2025
… Methods in SidePanePreferences and added the getSidePanePreferencesFromBackingStore method in JabRefGuiPreferences with proper updates in clear and importPreferences.
@Shreyasdone Shreyasdone marked this pull request as ready for review December 12, 2025 18:53
@github-actions github-actions bot removed the status: changes-required Pull requests that are not yet complete label Dec 12, 2025
Comment on lines +27 to +33
private SidePanePreferences() {
this(
Set.of(), // Default visible pane
Map.of(), // Default preferred positions
0 // Default web search fetcher index
);
}
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 correctly adapted:

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

private SidePanePreferences() {
    this(
            EnumSet.of(SidePaneType.WEB_SEARCH, SidePaneType.GROUPS), // Default visible panes (OPEN_OFFICE omitted)
            Collections.emptyMap(),                                  // Default preferred positions
            0                                                       // Default web search fetcher index
    );
}

Hi @calixtus , is this the approach that you're looking for?

Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

.

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Dec 12, 2025
@calixtus calixtus changed the title Refactoring SidePanePreferences to have default constructor and setAll method Reset and Import for SidePanePreferences Dec 12, 2025
@github-actions github-actions bot added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Dec 12, 2025
@Shreyasdone
Copy link
Contributor Author

Hi @calixtus, can you please check if the snippet I've added in the comments of the review is the one you want?

@Shreyasdone
Copy link
Contributor Author

Hi @koppor can you please check if the constructor is right in the reviews comments?

Comment on lines 712 to 713
getVisibleSidePanes(),
getSidePanePreferredPositions(),
Copy link
Member

Choose a reason for hiding this comment

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

These still need to be adapted too.

@Shreyasdone Shreyasdone requested a review from calixtus December 13, 2025 18:53
@github-actions github-actions bot removed the status: changes-required Pull requests that are not yet complete label Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: preferences good first issue An issue intended for project-newcomers. Varies in difficulty.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable resetting of SidePanePreferences

2 participants