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

Bring back the context menu #3666

Merged
merged 4 commits into from
Jan 29, 2018
Merged

Bring back the context menu #3666

merged 4 commits into from
Jan 29, 2018

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Jan 26, 2018

before after
before image

Removed a few commands that are not needed often and thus don't need to be placed that prominently.

  • Print entry preview: available through entry preview
  • All commands related to marking: marking is not yet reimplemented
  • Set/clear/append/rename fields: available through Edit menu
  • Manage keywords: available through Edit menu
  • Copy linked files to folder: available through File menu

Changes in the architecture:

  • Previously, an Action defined both the visual properties (text, icons, ...) as well as the behavior. These actions are usually defined in the base panel (or main frame).
  • Now the visual elements are defined by an ActionFX (to be renamed to Action as soon as the old class can be removed) and the behavior by implementing the Command interface. This separation makes it possible to reuse the visual aspects without the ties to the behavior.
  • In the long term, the commands should be defined outside of the main big classes. This is not done in this PR, because then the scope and size would explode. The extraction is best done peu à peu.

  • 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?)

@tobiasdiez tobiasdiez added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers maintable labels Jan 26, 2018
*/
public class OldCommandWrapper extends CommandBase {

private static final Log LOGGER = LogFactory.getLog(OldCommandWrapper.class);
Copy link
Member

Choose a reason for hiding this comment

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

Please use the new logging private static final Logger LOGGER = LoggerFactory.getLogger(OldCommandWrapper.class);

Copy link
Member Author

Choose a reason for hiding this comment

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

Your PR is not yet merged into javafxTable and I prefer to do this as soon as all the other maintable PRs are merged

Copy link
Member

Choose a reason for hiding this comment

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

Okay, just as a hint to make work then easier. Slf4j has a nice migrator tool (inlcuded in the zip download) https://www.slf4j.org/migrator.html to replace all occurences. Used that for the migration, too.

@Siedlerchr
Copy link
Member

Copy linked files to folder: moved to Tools menu
Already presend in the File menu
Open File PDF icon is missing

@tobiasdiez
Copy link
Member Author

@Siedlerchr Thank you for the feedback. I implemented your suggestions accordingly.

@tobiasdiez tobiasdiez merged commit 8980713 into javafxTable Jan 29, 2018
@tobiasdiez tobiasdiez deleted the contextMenu branch January 29, 2018 11:59
@koppor
Copy link
Member

koppor commented Apr 10, 2018

Can't we have an "Edit" submenu here containing all the edit actions available?

For me, this conforms to "Lernförderlichkeit" and "Erwartungskonformität" from EN ISO 9241-110 Grundsätze der Dialoggestaltung. Especially the "Erwawrungskonformität", because I expect that all actions available for the current entry are available in the context menu.

@koppor koppor added the ui label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintable status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants