Skip to content

Changes in the entry editor mark the database as dirty#2997

Closed
lenhard wants to merge 2 commits into
masterfrom
entry-editor-dirty
Closed

Changes in the entry editor mark the database as dirty#2997
lenhard wants to merge 2 commits into
masterfrom
entry-editor-dirty

Conversation

@lenhard

@lenhard lenhard commented Jul 11, 2017

Copy link
Copy Markdown
Member

Partial fix to #2787

Changes in the entry editor now mark the database as dirty. This are somewhat proactive: even only opening the entry editor will mark the database as changed. I'd say this isn't a problem. I'd rather have it do the marking than forget one.

Essentially, this adds another listener to the properties in the entry editor that propagates the change whenever the property changes. I haven't figured out how to do this for groups.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@lenhard lenhard added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 11, 2017
@stefan-kolb

Copy link
Copy Markdown
Member

I'd say this is a hack. Whereas it is better than missing a changed state for now, we should only mark the DB as dirty if we really changed something.

@tobiasdiez

Copy link
Copy Markdown
Member

I'm also not very happy with this solution from an architectural view: why should the gui handle the dirty/clean state and not the logic? I would propose to add listeners to the database/metadata in DatabaseContext which sets a corresponding flag. The UI can then listen to changes of this new property and display the star in the tab accordingly.

@Siedlerchr

Copy link
Copy Markdown
Member

The databaseChangedEvent is already realized with the EventBus system, so it just needs a couple of listeners if I understand it correctly. And one event bus can have multiple listeners for the same event

@lenhard

lenhard commented Jul 11, 2017

Copy link
Copy Markdown
Member Author

Ok, I see. So far it had been implemented only in the BasePanel, but I aggree that the cleanest solution is probably to track it in the database itself. This won't be as quick as the current solution, though :)

The consensus seems to be that we are not going in this direction, so I'll close this PR.

@lenhard lenhard closed this Jul 11, 2017
@stefan-kolb stefan-kolb deleted the entry-editor-dirty branch July 20, 2017 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants