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

Refactoring the UI for improved Connector/Context/Action display #484

Closed
frankiejarrett opened this issue Apr 30, 2014 · 15 comments
Closed

Comments

@frankiejarrett
Copy link
Contributor

During WCATX, I sat down with @lukecarbis for a couple of hours and we hashed out a plan to greatly improve our display of Connectors, Contexts and Actions without any significant changes to the data architecture.

Contexts need more context

Over time there have been issues raised about contextual help (#377 and #476) being needed when choosing a Context.

In an ironic twist, it seems that the Context names are lacking context in and of themselves. For instance "Video" could be a context label for Media connector because it's an attachment type, but it could also be a post type name in the Posts connector.

The result is that you could end up with two Contexts labels with the same name with no way to tell them apart. Not cool.

Multiple contexts for records aren't needed

Another thing that has been learned about Stream is that Connectors and Contexts really do exist in one-to-one pairs on records.

It was decided early on by @shadyvb and myself that we allow for multiple contexts to be added to a record, but thus far, we have not found any use case for this to be necessary.

It seems logical now that Connectors and Contexts should be displayed as a paring in the UI, with Connectors being the top-level group and Contexts being the sub-level items with those groups. Again, I'm only talking about the UI.

EXAMPLE

screen shot 2014-04-29 at 3 21 47 am

Action and Context relationships

Actions have always been non-hierarchal. This is good. It allows you to see all "Updated" activity without requiring a Connector or Context to be chosen.

However, we've realized that this doesn't mean there shouldn't be any relationship at all for UI purposes. For instance, if you first select "Created" as an Action and then look at the Context dropdown, you should only see those Contexts that support the selected Action.

The idea would be to convert the get_action_lables() method into get_actions() with each action having an associative array of args, like label and contexts. This would allow us to create the relationships we need to dynamically change the UI based on what is selected.

EXAMPLE

public static function get_actions() {
    return array(
        'updated'         => array(
            'label'    => __( 'Updated', 'stream' ),
            'contexts' => array( 'profiles' ),
        ),
        'created'         => array(
            'label'    => __( 'Created', 'stream' ),
            'contexts' => array( 'users' ),
        ),
        'deleted'         => array(
            'label'    => __( 'Deleted', 'stream' ),
            'contexts' => array( 'profiles' ),
        ),
        'password-reset'  => array(
            'label'    => __( 'Password Reset', 'stream' ),
            'contexts' => array( 'profiles' ),
        ),
        'forgot-password' => array(
            'label'    => __( 'Forgot Password', 'stream' ),
            'contexts' => array( 'profiles' ),
        ),
        'login'           => array(
            'label'    => __( 'Login', 'stream' ),
            'contexts' => array( 'sessions' ),
        ),
        'logout'          => array(
            'label'    => __( 'Logout', 'stream' ),
            'contexts' => array( 'sessions' ),
        ),
        'failed_login'    => array(
            'label'    => __( 'Failed Login', 'stream' ),
            'contexts' => array( 'sessions' ),
        ),
    );
}
@westonruter
Copy link
Contributor

It was decided early on by @shadyvb and myself that we allow for multiple contexts to be added to a record, but thus far, we have not found any use case for this to be necessary.

I toyed with this in #391 for the action of moving a widget from one area to another. I was initially adding two contexts for the action, so two records would appear in the list but with the two sidebars as the respective actions, with the same Stream record ID appearing for each. Since then, however, I've changes from a moved action to two actions: removed and added, but I've left an open question on the issue:

When moving a widget from one sidebar to another, should there be one Stream record with two contexts, or two records with one context each?

@frankiejarrett
Copy link
Contributor Author

@westonruter Would there really be two Contexts, or would it actually be two Actions?

@westonruter
Copy link
Contributor

That's the question. Does a "move" action have have its context on the source, the destination, or on the thing being moved?

@shadyvb
Copy link
Contributor

shadyvb commented Apr 30, 2014

@westonruter Exactly, that's the question, i do strongly think that an operation can be attributed to many contexts/connectors relatively according to what a user would think. BUT, it'll always belong to a single object, therefore, and since we should be stating facts not relativity to other contexts, i say we drop having multiple records/contexts for a single action.
Example for my point: When i put a woocommerce widget in 'Home sidebar' i'm altering a sidebar, the homepage, and a woocommerce widget, many connections here, but it'll always be 'Putting a widget in a sidebar', all other connections can always be traced down via record data ( and meta ).

@frankiejarrett
Copy link
Contributor Author

@shadyvb So to answer @westonruter's question, there should be multiple records created if the action performed by the user applies to more than one context. Right?

@shadyvb
Copy link
Contributor

shadyvb commented Apr 30, 2014

@fjarrett I'm leaning towards not creating multiple records for a single action. No multiple contexts, and no multiple records. Happy to discuss any use case that would require doing so though.

@frankiejarrett
Copy link
Contributor Author

@shadyvb So if you move a widget from Page Sidebar to Home Sidebar, do we log that it was "Removed from Page Sidebar", "Added to Home Sidebar" or "Moved from Page Sidebar to Home Sidebar"?

@shadyvb
Copy link
Contributor

shadyvb commented Apr 30, 2014

@fjarrett Yes, Moved from X to Y, since we're tracking the object itself, the widget here, and not sidebars. That is what Stream was doing ( at least adding proper meta for old/new sidebars ).

However, the issue behind what @westonruter introduced, IMO, was to be able to filter actions that happened to a sidebar by simply filtering by its context. That might be a good use case, but three points here to make:

  1. If i want to track widget actions, i can filter by connector, there shouldn't be much records there as it is not a frequently changed part of any website. So i don't really need to filter by context/sidebar.
  2. An admin can always query actions that happened to a specific sidebar via meta-queries. [We might need to think about that, enabling admins to include meta-queries as filters in Stream list-table]. OR, he can search in summary via the sidebar name if we include both sidebar names in there.
  3. Average users would not want to see duplicate records for a single action
  4. Couple of extensions ( Reports, Gamify, Notifications, ... ) would not feel comfortable about duplicate records for a single action as well.

Apart from that, we do need to set a ground rule about what a record actually belong to. So we have a rule to apply when such issues occur in the future.

/cc @westonruter

@westonruter
Copy link
Contributor

So then should #391 be updated to only use the destination sidebar as the context?

@frankiejarrett
Copy link
Contributor Author

@westonruter That's correct, and the old sidebar would be stored as meta for use in the summary.

@lukecarbis
Copy link
Contributor

Would it be worth considering in this thread the idea of having no context? For example, when a comment is added, the connector is Comment and the context is Comment. Or when a setting with no settings screen is updated, both it's connector and context is Settings. This double-up might be confusing for an average user.

Would allowing multiple contexts or zero contexts break this UI improvement?

To me, the best case scenario when moving the About Me widget from Primary Sidebar to Secondary Sidebar is:

  • Summary: "About Me" widget moved from Primary Sidebar to Secondary Sidebar
  • Connector: Widgets
  • Context: Primary Sidebar and Secondary Sidebar,
  • Action: Moved

Whether or not that best case scenario is viable, I don't know.

mz

Implementing Contexts as an array might also allow for sub-contexts. For example, when updating the PayPal email payment gateway setting in WooCommerce, it might look like this:

WooCommerce (connector)
    > Settings
        > Checkout
            > PayPal

@frankiejarrett
Copy link
Contributor Author

@lukecarbis I think one of the goals in this issue is to decide if multiple contexts on a record is necessary, and so far I think we're leaning towards no. This will mean that the stream_context table can be completely deleted! Making queries even faster and reducing the DB by a third(-ish).

I love your idea here though about no context, but instead of making any actual change to the way the data is stored we could just handle this in the UI only, by hiding the second-level context if the label matches the connector name. So it would appear there is no context, but really, in the DB there is.

@lukecarbis
Copy link
Contributor

@fjarrett No problem. Working on this now.

@shadyvb shadyvb mentioned this issue May 8, 2014
11 tasks
@lukecarbis
Copy link
Contributor

Just putting this here for posterity.

I've hit a point on branch issue-484 where I need to rethink how my code is working. My initial approach was to use optgroups, and ask select2 to make optgroup labels selectable.

After getting there, I've realised that this creates a few other problems. The biggest being that I can't set the value using javascripts .val(). What seemed like the right approach to begin with is starting to feel like a hack.

So my new approach will be to simply use styling to differentiate between a level 1 item and a level 2 item in the list.

@lukecarbis
Copy link
Contributor

Done! It will be in the 2.0.0 release.

@frankiejarrett frankiejarrett added this to the 2.0.0 milestone Jun 5, 2014
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.

4 participants