Skip to content

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

Merged
koppor merged 9 commits into
JabRef:mainfrom
DumitruDruta:fix-for-issue56
Jan 9, 2024
Merged

Changed default status of 'Automatically open folder of attached files'#10748
koppor merged 9 commits into
JabRef:mainfrom
DumitruDruta:fix-for-issue56

Conversation

@DumitruDruta

@DumitruDruta DumitruDruta commented Jan 5, 2024

Copy link
Copy Markdown
Contributor

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.

public void setValues() {
eMailReferenceSubjectProperty.setValue(initialExternalApplicationPreferences.getEmailSubject());
autoOpenAttachedFoldersProperty.setValue(initialExternalApplicationPreferences.shouldAutoOpenEmailAttachmentsFolder());
autoOpenAttachedFoldersProperty.setValue(!initialExternalApplicationPreferences.shouldAutoOpenEmailAttachmentsFolder());

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@DumitruDruta

DumitruDruta commented Jan 6, 2024

Copy link
Copy Markdown
Contributor Author

Changelog has been updated.

Siedlerchr
Siedlerchr previously approved these changes Jan 6, 2024
Comment thread CHANGELOG.md Outdated
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

koppor commented Jan 8, 2024

Copy link
Copy Markdown
Member

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

image

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

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
Copy Markdown
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

koppor commented Jan 9, 2024

Copy link
Copy Markdown
Member

@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
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