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

Changed default status of 'Automatically open folder of attached files' #10748

Merged
merged 9 commits into from
Jan 9, 2024

Conversation

DumitruDruta
Copy link
Contributor

@DumitruDruta DumitruDruta commented Jan 5, 2024

I have changed the default value of the "Automatically open folders of attached files" checkbox to checked. This has been done by negating the boolean value that controls this checkbox and the feature behind it. "Closes JabRef#56"
issue56

Mandatory checks

  • 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.

@@ -90,7 +90,7 @@ public ExternalTabViewModel(DialogService dialogService, PreferencesService pref
@Override
public void setValues() {
eMailReferenceSubjectProperty.setValue(initialExternalApplicationPreferences.getEmailSubject());
autoOpenAttachedFoldersProperty.setValue(initialExternalApplicationPreferences.shouldAutoOpenEmailAttachmentsFolder());
autoOpenAttachedFoldersProperty.setValue(!initialExternalApplicationPreferences.shouldAutoOpenEmailAttachmentsFolder());
Copy link
Member

Choose a reason for hiding this comment

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

this does not make sense. Whenever the user checks the checkbox, it will not be opened?

You just need to adjust it in the JabRefPreferences itself, change defaults value for the property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your feedback. I have taken it into consideration and made some changes, which I already pushed. I hope the new solution will be better.

@Siedlerchr
Copy link
Member

looks good, now please add a changelog entry to the "Changed section"

@DumitruDruta
Copy link
Contributor Author

DumitruDruta commented Jan 6, 2024

Changelog has been updated.

Siedlerchr
Siedlerchr previously approved these changes Jan 6, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
Siedlerchr
Siedlerchr previously approved these changes Jan 6, 2024
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 6, 2024
@koppor
Copy link
Member

koppor commented Jan 8, 2024

Thank you for the effort. Unfortunately, this PR DOES NOT solve the issue.

image

The issue is about introducing an Operating-System specific default!!

@@ -737,7 +737,7 @@ private JabRefPreferences() {
defaults.put(KEY_GEN_ALWAYS_ADD_LETTER, Boolean.FALSE);
defaults.put(EMAIL_SUBJECT, Localization.lang("References"));
defaults.put(KINDLE_EMAIL, "");
defaults.put(OPEN_FOLDERS_OF_ATTACHED_FILES, Boolean.FALSE);
defaults.put(OPEN_FOLDERS_OF_ATTACHED_FILES, Boolean.TRUE);
Copy link
Member

Choose a reason for hiding this comment

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

Please make this OS-specific.

If Windows: TRUE - else FALSE

@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 8, 2024
@koppor koppor enabled auto-merge January 9, 2024 10:09
@koppor
Copy link
Member

koppor commented Jan 9, 2024

@DumitruDruta Thank you for the quick follow-up. We will merge as soon as the checks pass.

@koppor koppor added this pull request to the merge queue Jan 9, 2024
Merged via the queue into JabRef:main with commit b45cc01 Jan 9, 2024
18 checks passed
Siedlerchr added a commit that referenced this pull request Jan 12, 2024
* upstream/main: (25 commits)
  Remove remainging "testLogging" (#10769)
  Reuse JUnit 5.7's @DisabledIfEnvironmentVariable (#10768)
  Refactored LibraryTab (#10732)
  Fix annotation (#10766)
  Add initial .git-blame-ignore-revs (#10765)
  Streamline test output (#10762)
  Fix NPE when opening and re-opening a library (#10763)
  Changed default status of 'Automatically open folder of attached files' (#10748)
  Implement test cases for search (#10193)
  Add cites field to bib entries for citation relation (#10752)
  Ignore submodule changes (for normal devs) (#10754)
  Fix line endings
  Bump lycheeverse/lychee-action from 1.8.0 to 1.9.0 (#10760)
  Bump appleboy/ssh-action from 1.0.2 to 1.0.3 (#10761)
  Bump org.apache.lucene:lucene-core from 9.9.0 to 9.9.1 (#10759)
  Bump org.openrewrite.rewrite from 6.5.4 to 6.6.3 (#10758)
  Bump org.glassfish.jersey.containers:jersey-container-grizzly2-http (#10755)
  Bump org.jsoup:jsoup from 1.16.2 to 1.17.2 (#10756)
  Bump org.mockito:mockito-core from 5.7.0 to 5.8.0 (#10757)
  Remove abstract
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default Preferences: Windows: Open folder when sending email
3 participants