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.

Sign up for GitHub

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

Jump to bottom

Fix for resizing cleanup entries dialog #9240

Merged
merged 4 commits into from
Oct 15, 2022
Merged

Fix for resizing cleanup entries dialog #9240

merged 4 commits into from
Oct 15, 2022

Conversation

waterflow80
Copy link
Contributor

@waterflow80 waterflow80 commented Oct 11, 2022 •
edited by Siedlerchr
Loading

Fixes #9223

Issue description

The content of the dialog for the cleanup entries becomes partially visible when resizing the
dialog (decreasing the size) vertically or horizontally.

Solution description

I added a ScrollPane and put the content of the presetPanel inside this ScrollPane in order to
be able to scroll and view the rest of the content when needed (when the dialog is resized).

Note: I put the name of the ScrollPane Object as scrollPane but I believe the name can be more significant and coherent
with the project structure.

I also changed the preferred height to 650, so when the dialog opens, it displays all the initial information without the need
to display the scroll bar.

The new CleanupDialog

Screenshot from 2022-10-11 23-43-07

Other possible enhancements

While fixing this issue, i noticed that the CleanupDialog can be resized with no limits. In other words the size of the dialog
can be reduced to 0 or extended to infinity, as show the pictures below:

Screenshot from 2022-10-12 00-00-43

Screenshot from 2022-10-12 00-07-17

So we can add min-width and min-height for the Dialog.

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

@Siedlerchr
Copy link
Member

Thanks for the PR. Please have a look at the failing test checkstyle or the reviewdog output.
Please also adjust the title of your PR so that it is directly clear what is about

@Siedlerchr Siedlerchr changed the title Fix for issue 9223 Fix for resizing cleanup entries dialog Oct 12, 2022
@ThiloteE ThiloteE added the ui label Oct 12, 2022
@waterflow80 waterflow80 reopened this Oct 12, 2022
@waterflow80
Copy link
Contributor Author

waterflow80 commented Oct 12, 2022 •
edited
Loading

Thanks for the PR. Please have a look at the failing test checkstyle or the reviewdog output. Please also adjust the title of your PR so that it is directly clear what is about

Hi @Siedlerchr, Thank you for you feedback,
I see that there is a failed check Cleanup after PR / cleanup (pull_request), and I don't really understand why it did that,
and whether it is required to pass in order to qualify for the merge.
Screenshot from 2022-10-12 21-04-13

And I am sure if I am concerned with the Deployment / Create installer and portable version for macOS (pull_request) check fail.

@Siedlerchr
Copy link
Member

Hi,
You closed the PR so the action Cleanup after PR was triggered. But it cannot execute because it requires some secrets which are not avaiable in forks.
The same applies for mac os. It need some signing keys, however these are not avaiable on forks

@waterflow80
Copy link
Contributor Author

Hi,
You closed the PR so the action Cleanup after PR was triggered. But it cannot execute because it requires some secrets which are not avaiable in forks.
The same applies for mac os. It need some signing keys, however these are not avaiable on forks

@Siedlerchr Yes I apologize because I mistakenly closed it (and eventually reopened it).
So I think that my PR is now ready for review?

@HoussemNasri HoussemNasri added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 12, 2022
calixtus
calixtus reviewed Oct 13, 2022
View reviewed changes
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.

Changes look good so far to me, except one thought I would just like to discuss or think about.... Comments on this?

src/main/java/org/jabref/gui/cleanup/CleanupDialog.java Show resolved Hide resolved
@Siedlerchr
Copy link
Member

Thanks for your contribution! I think it's okay with the height then

@Siedlerchr Siedlerchr merged commit 194d792 into JabRef:main Oct 15, 2022
@waterflow80
Copy link
Contributor Author

Thanks for your contribution! I think it's okay with the height then

@Siedlerchr It was a pleasure for me. Thanks.

Siedlerchr added a commit that referenced this pull request Oct 17, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into default_file_dir
c2597e7 
* upstream/main:
  Make autosave tick in shared database opening dialog active (#9258)
  Bump controlsfx from 11.1.1 to 11.1.2 (#9261)
  Bump actions/configure-pages from 1 to 2 (#9262)
  Bump hmarr/auto-approve-action from 2.4.0 to 3.0.0 (#9259)
  Bump gittools/actions from 0.9.13 to 0.9.14 (#9260)
  Fix for resizing cleanup entries dialog (#9240)
  Refresh example styles
  Squashed 'buildres/csl/csl-locales/' changes from cb98d36691..e243665390
  Squashed 'buildres/csl/csl-styles/' changes from 7bde3e4..4eee79a
  Fix casing
  Update contributing.md
  Remove obsolete Jekyll howto
  Updates Just-the-Docs from 0.3.3 to 0.4.0.rc3 (#9249)
  Place subgroups w.r.t. alphabetical ordering (#9228)
  DOAB Fetcher now fetches ISBN and imprint fields where possible (#9229)
  ISSUE-9145: implement isbn fetcher (#9205)
  Remove explicit javafx jmod stuff (#9245)
  New Crowdin updates (#9239)
Siedlerchr added a commit that referenced this pull request Oct 21, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-9221
55303f1 
* upstream/main: (28 commits)
  Minor code improvements in library properties dialog (#9265)
  Adjust and make tests for BibTex/Biblatex/CSL mapping more accurate by adding Apa 7th edition (#9255)
  Removed swing from default file dir detection (#9222)
  Make autosave tick in shared database opening dialog active (#9258)
  Bump controlsfx from 11.1.1 to 11.1.2 (#9261)
  Bump actions/configure-pages from 1 to 2 (#9262)
  Bump hmarr/auto-approve-action from 2.4.0 to 3.0.0 (#9259)
  Bump gittools/actions from 0.9.13 to 0.9.14 (#9260)
  Fix for resizing cleanup entries dialog (#9240)
  Refresh example styles
  Squashed 'buildres/csl/csl-locales/' changes from cb98d36691..e243665390
  Squashed 'buildres/csl/csl-styles/' changes from 7bde3e4..4eee79a
  Fix casing
  Update contributing.md
  Remove obsolete Jekyll howto
  Updates Just-the-Docs from 0.3.3 to 0.4.0.rc3 (#9249)
  Place subgroups w.r.t. alphabetical ordering (#9228)
  DOAB Fetcher now fetches ISBN and imprint fields where possible (#9229)
  ISSUE-9145: implement isbn fetcher (#9205)
  Remove explicit javafx jmod stuff (#9245)
  ...
Siedlerchr added a commit to TheGor1lla/jabref that referenced this pull request Oct 21, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-9214
d49632a 
* upstream/main:
  [WIP] Add GitHub action: Greetings.yml - Automates advice to JabRefs first time code contributors (JabRef#9272)
  Minor code improvements in library properties dialog (JabRef#9265)
  Adjust and make tests for BibTex/Biblatex/CSL mapping more accurate by adding Apa 7th edition (JabRef#9255)
  Removed swing from default file dir detection (JabRef#9222)
  Make autosave tick in shared database opening dialog active (JabRef#9258)
  Bump controlsfx from 11.1.1 to 11.1.2 (JabRef#9261)
  Bump actions/configure-pages from 1 to 2 (JabRef#9262)
  Bump hmarr/auto-approve-action from 2.4.0 to 3.0.0 (JabRef#9259)
  Bump gittools/actions from 0.9.13 to 0.9.14 (JabRef#9260)
  Fix for resizing cleanup entries dialog (JabRef#9240)
  Refresh example styles
  Squashed 'buildres/csl/csl-locales/' changes from cb98d36691..e243665390
  Squashed 'buildres/csl/csl-styles/' changes from 7bde3e4..4eee79a
  Fix casing
  Update contributing.md
  Remove obsolete Jekyll howto
  Updates Just-the-Docs from 0.3.3 to 0.4.0.rc3 (JabRef#9249)
  Place subgroups w.r.t. alphabetical ordering (JabRef#9228)
  DOAB Fetcher now fetches ISBN and imprint fields where possible (JabRef#9229)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThiloteE ThiloteE ThiloteE left review comments

@calixtus calixtus calixtus left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cleanup entries dialog is partially visible
5 participants
@waterflow80 @Siedlerchr @calixtus @ThiloteE @HoussemNasri