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

Implement actions api #68

Closed
weisJ opened this issue Oct 9, 2020 · 4 comments · Fixed by #83
Closed

Implement actions api #68

weisJ opened this issue Oct 9, 2020 · 4 comments · Fixed by #83

Comments

@weisJ
Copy link
Contributor

weisJ commented Oct 9, 2020

Provide an api to declare actions which will then be consumed by the GUI.
This is related to #67 by depending on it.

There are several places where actions can come up in the gui:

  • In the menubar.
  • In the toolbar.
  • (Implicitly as keyboard shortcuts)

Lets call them ActionViewType for now. I did put keyboard shortcuts into parenthesis because those could be realised by binding them to one of the other view types.

Additionally to the view types the actions serve they should be hierarchical structure with some sort of ActionGroup.
Grouping would then take into effect when constructing the menubar.

ActionGroups have the following properties:

  • A name
  • An optional description
  • An optional icon
  • An indicator whether they are enabled (e.g. a possible EditorActionGroup would be disabled if no project is loaded).

Additionally Actions have:

  • An optional key binding
  • Still an indicator whether they are enabled (e.g. undo/redo con only work if there is something to undo/redo).

Actions should be loaded through the service-loader.

@weisJ
Copy link
Contributor Author

weisJ commented Oct 11, 2020

Something else to consider is how we manage sorting of actions. Because we are loading actions as a service their order isn't guaranteed to be the same between builds.

We could sort them alphabetically but this wouldn't make sense for an action group containing "Undo" and "Redo" (which should be sorted in this order).

Then again how do we sort groups in their hierarchy i.e. in the top level group "Edit" (Which corresponds to the "Edit" menu in the menubar we most likely want the "Undo/Redo" group to show up at the top of the popup menu.

Maybe we can get away with some sort of "Priority" flag for actions and action groups wherein we first sort by priority and then by some comparator the action group provides (alphabetical would be the default for groups which are unaware of their content).

The example above could then look something like this:

  • Edit Group (Sorting = Priority first then alphabetical)
    • Undo/Redo Group (Priority = High, Sorting = [Undo, Redo])
      • Redo Action
      • Undo Action
    • Some other group B (Priority = Default, Sorting = Priority first then alphabetical)
      • Action B.1 (Priority = Low)
      • Action B.2 (Priority = Default)
      • Action B.3 (Priority = High)
    • Some other group A (Priority = Default, Sorting = Priority first then alphabetical)
      • Action A.1 (Priority = Default)
      • Action A.2 (Priority = High)
    • Some standalone action B (Priority = Default)
    • Some standalone action A (Priority = Default)
    • Some standalone action C (Priority = High)

Priority = Default is implicit for all groups and actions. For groups Sorting = Priority first then alphabetical is implicit.
Because the standalone actions don't belong to any intermediate group they implicitly belong to the "unnamed_group" which has Priority = Lowest and Sorting = Priority first then alphabetical. The resulting ordering then will be

  • Edit Group
    • Undo/Redo Group
      • Undo Action
      • Redo Action
    • Some other group A
      • Action A.2
      • Action A.3
    • Some other group B
      • Action B.3
      • Action B.2
      • Action B.1
  • Some standalone action C
  • Some standalone action A
  • Some standalone action B

For consistency the alphabetical ordering shouldn't be dictated by the display name but rather an internal identifier of the action (group).


I'm not fully satisfied with this approach as I'm unsure whether it gives us enough flexibility in controlling menu structure and I'd be happy to get some feedback on it.

@marko-radosavljevic
Copy link
Collaborator

marko-radosavljevic commented Oct 12, 2020

I think we shouldn't worry too much about details like this. It's impossible to get design perfectly right the first time, code is meant to be changed and rewritten. We should just focus on making those changes as easy as possible. Action order changes rarely, and nothing really depends on it, so fixing design problems around it should be trivial. So sorting by priority is fine, until we find a better solution.

That said, I'm struggling to connect actions to GUI components. It would be nice if you would walk me through everything event/action/component/DI related from the application start. I guess I'm mostly confused with the meaning of 'action'. I assume 'action' is some action user performed, like button click/press, so 'action' is component specific action that generates event that we broadcast over the bus to everyone interested? Or action is piece of code that processes an event (eg. something that would implement ActionListener)?

@weisJ
Copy link
Contributor Author

weisJ commented Oct 12, 2020

I guess I'm mostly confused with the meaning of 'action'

The actions are "actions" that manipulate the applications model. Eve though button click/press is an action in the world of swing, we delegate those to our action classes.


I patched together an example of how I imagine all the APIs will go together.
It's just a simple counter which can be incremented using a button as long as the current counter value is less than 10.

Some general notes:

  • The EventBus API is rather low level and used by higher level APIs. Most parts of the application won't have to deal with it directly.
  • The annotations @Model and @ViewProviderFor aren't fully worked out yet. Their exact semantics depend on the implementation of the Application class returned by the ApplicationManager.
  • In the world of MVC:
    • CounterModel is the model.
    • CounterViewProvider "is" the view. (Depending on the implementation of @ViewProviderFor there might be no createView method at all).
    • Actions (i.e. AnAction) are the controllers. Of course more complex views and models may need an intermediate controller which will be bound to the view. Smaller controllers though are simply wired when creating the view itself.
  • We will need some bridge classes to mediate between the models of swing (i.e. for SelectionModel, TableModel, TreeModel etc.) and our own models
    @AutoService(AppComponent.class)
    class CounterAppComponent implements AppComponent {

        private final CounterModel model;

        public CounterAppComponent() {
            model = ApplicationManager.getApplication().getModel(CounterModel.class);
        }

        @Override
        public String getTitle() {
            return "Counter";
        }

        @Override
        public Alignment getPreferredPosition() {
            return Alignment.NORTH_WEST;
        }

        @Override
        public JComponent getComponent() {
            return UIViewFactory.createView(model);
        }
    }


    @Model
    class CounterModel implements Observable {

        public static final String COUNT_PROPERTY = "count";

        private IntProperty count = new IntProperty();

        public int getCount() {
            ServiceLoader.loadInstalled()
            return count.getValue();
        }

        public void setCount(final int count) {
            count.setValue(count);
        }
    }

    @AutoService(AnAction.class)
    class IncrementAction implements AnAction {

        @Override
        public String getIdentifier() {
            return "increment_counter";
        }

        @Override
        public String getDisplayName() {
            return "Increment";
        }

        @Override
        public boolean isEnabled() {
            var model = getCounterModel();
            return model != null && model.getCount() <= 10;
        }

        @Override
        public void runAction() {
            var model = getCounterModel();
            if (model == null) return;
            model.setCount(model.getCount() + 1);
        }

        private CounterModel getCounterModel() {
            return ApplicationManager.getApplication().getModelIfCreated(CounterModel.class);
        }
    }

    @ViewProviderFor(model = CounterModel.class)
    class CounterViewProvider implements ViewProvider<CounterModel> {

        @Override
        public JComponent createView(final CounterModel model) {
            JLabel counterLabel = new JLabel();
            model.observe(CounterModel.COUNT_PROPERTY, (oldValue, newValue) -> {
                counterLabel.setText(Objects.toString(newValue));
            });
            JPanel holder = new JPanel(new BorderLayout());
            JPanel labelHolder = new JPanel(new GridBagLayout());
            labelHolder.add(counterLabel);
            holder.add(counterLabel, BorderLayout.CENTER);

            JComponent buttonPanel = Box.createHorizontalBox();
            buttonPanel.add(Box.createHorizontalGlue());

            JButton incrementButton = new JButton("Increment Counter");

            // Because this would be a common pattern this could be extracted into a helper method:
            incrementButton.addActionListener(e -> {
                EventBus.get(Topic.ACTIONS).post("increment_counter");
            });

            buttonPanel.add(incrementButton);
            buttonPanel.add(Box.createHorizontalGlue());

            return holder;
        }
    }

@marko-radosavljevic
Copy link
Collaborator

Ah, thanks, now I have a full picture. I was thinking on different level of abstraction, more about event bus than high level MVC implementation. 👍

@weisJ weisJ linked a pull request Oct 17, 2020 that will close this issue
@weisJ weisJ closed this as completed in #83 Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants