Skip to content

Fix opening of entry editor#4784

Merged
tobiasdiez merged 1 commit into
masterfrom
fixEditor
Mar 19, 2019
Merged

Fix opening of entry editor#4784
tobiasdiez merged 1 commit into
masterfrom
fixEditor

Conversation

@tobiasdiez

@tobiasdiez tobiasdiez commented Mar 19, 2019

Copy link
Copy Markdown
Member

Fixes #4762. The problem was that the NotificationPane was hiding the entry editor/preview.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@tobiasdiez
Fix opening of entry editor
2af18ad 
Fixes #4762. The problem was that the NotificationPane was hiding the entry editor/preview.
@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 19, 2019
matthiasgeiger
matthiasgeiger suggested changes Mar 19, 2019
View reviewed changes

@matthiasgeiger matthiasgeiger left a comment •
edited
Loading

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.

Ahh... I also investigated the problem and came up with a similar solution, but had still some problems.

I tested your change now and there are also some issues remaining:

  1. after creating a new database the screen is completely empty not showing the main table
  2. after opening the preferences and saving, I cannot close the Entry editor anymore

... but I do not really understand why these aspects are currently failing

//Edit: I pushed my proposal to branch fix-4762 if you want to have a look

Siedlerchr
Siedlerchr approved these changes Mar 19, 2019
View reviewed changes

@Siedlerchr Siedlerchr left a comment

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.

Lgtm. Just unsure if the show and edit is not called from a background thread somewhere. Could be the case that all are converted now

@tobiasdiez

tobiasdiez commented Mar 19, 2019

Copy link
Copy Markdown
Member Author

@matthiasgeiger sorry for the bad timing, I hope you didn't investigated too much time into this bug. You are right, these things should be fixed as well. Since I'll have on time to have a look at these issues in the next days but the bug addressed in this PR is mayor, I'll merge now but leave #4762 open as a reminder.

@tobiasdiez tobiasdiez merged commit 90376f4 into master Mar 19, 2019
@tobiasdiez tobiasdiez deleted the fixEditor branch March 19, 2019 22:29
@tobiasdiez tobiasdiez mentioned this pull request Mar 19, 2019
Closed
1 task
matthiasgeiger added a commit that referenced this pull request Mar 24, 2019
@matthiasgeiger
followup of #4784 - fixes remaining issues of #4762
73bfc78
@matthiasgeiger matthiasgeiger mentioned this pull request Mar 24, 2019
Merged
2 tasks
Siedlerchr pushed a commit that referenced this pull request Mar 25, 2019
@matthiasgeiger @Siedlerchr
Followup of #4784 - fixes remaining issues of #4762 (#4806)
e14b567 
* followup of #4784 - fixes remaining issues of #4762

* use Path instead of File

Co-authored-by: matthiasgeiger <matthias.geiger@uni-bamberg.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

+1 more reviewer

@matthiasgeiger matthiasgeiger matthiasgeiger requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tobiasdiez @Siedlerchr @matthiasgeiger