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

Spec for concept-b changes #256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

gnehapk
Copy link
Member

@gnehapk gnehapk commented Jan 23, 2018

  • List all the changes required in concept-b implementation
  • Dependencies for it

@gnehapk
Copy link
Member Author

gnehapk commented Jan 23, 2018

@r0h4n r0h4n added this to the Milestone 1 (2018) milestone Feb 5, 2018
r0h4n
r0h4n previously approved these changes Feb 5, 2018
@r0h4n
Copy link
Contributor

r0h4n commented Feb 5, 2018

@julienlim please review this, we need to close this

@julienlim
Copy link
Member

julienlim commented Feb 5, 2018

@gnehapk @nthomas-redhat @r0h4n @shtripat @julienlim @mcarrano @mbukatov @a2batic @cloudbehl @jjkabrown1

In reviewing the spec, I'd like to suggest adding the following (I'm not able to edit the spec as I don't have permissions to do so):

Introduction
The intent of this change is to optimize the user experience and usability for the Tendrl users, i.e. allowing quicker access to managed objects through fewer steps, as well as allowing for dynamic product features (menu items) to be exposed based on a given context (cluster type). Moreover, it allows for easier access to top-level objects (cluster resources) being managed with a given cluster.

Problem Description
The Tendrl release 1.5.4 uses a navigation that requires multiple-level drill-down to examine details of objets with a cluster. Its navigation uses a static menu bar on the left with access to top-level objects only via drill-downs in the right pane, which is less than optimal.

  • It’s not easy access to access cluster-top level objects without a drill-down, and it’s not obvious to user where they go to find volume information a quick glance.
  • To switch to another cluster from a drill-down, a user has to go to the Cluster via the menu item or breadcrumb to choose a different cluster (2 step/clicks).

To optimize and improve the user experience and usability, we want to introduce a more optimal navigation, whereby we use a Context Selector in the masthead to allow for quickly switching between clusters. Additionally, the left-hand navigation menu bar would be context-sensitive, i.e. the menu is shown in context of a selector cluster, allowing for different menu items (product features) to be presented for different cluster types (e.g. Ceph and Gluster). No navigation bar is shown when all clusters are selected. This is further detailed in Concept B: Single Cluster / Element Manager Concept with Context Switcher Navigation (Optimized UX).

Performance Impact
There should be no UI performance impact due to this change.

Acceptance Criteria
The following should be satisfied to confirm that this spec is completed and working as intended:

  • The left-navigation bar should only appear when a single cluster is selected in the Context Selector or from any place where a single cluster is selected, e.g. breadcrumb, hyperlink, etc.
  • When "All Clusters" is selected, there should be no left-navigation bar.
    ...
    ...

@julienlim
Copy link
Member

@gnehapk Thanks for adding the items I mentioned. I was hoping you would fill out the rest of the Acceptance Criteria (this is needed for QE to do their testing).

As I don't have push access to the spec, can you please add the rest of the following items under the Acceptance Criteria section? Once they are added, I'll approve the spec.

  • Verify Admin area is launched from the utilities in the top right and only accessible by Admin user.
  • Verify that hosts listing for a failed Import Cluster is showing in a modal (dialog).
  • Verify that Cluster, Host, and Volume details are hyperlinked from the object name (and not from a “Details” button)
  • Verify buttons in the Clusters, Hosts, and Volumes list views show primary actions and kebab menu to show secondary actions when there are more than 3 actions.
  • Verify volumes count in Clusters list view.
  • Verify bricks count in Hosts list view.
  • Verify bricks count in Volumes list view.

@r0h4n @Tendrl/tendrl-qe @mcarrano

Copy link
Member

@julienlim julienlim left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for making all the updates.

@ltrilety
Copy link

ltrilety commented Feb 8, 2018

Just a note. In current version there's no any 'View Details' instead there's 'Track Progress' when the import is running.

image

@julienlim
Copy link
Member

@ltrilety Noted with the recent 1.5.5 release. Can you please file a GitHub issue for it? Thanks.

@ltrilety
Copy link

ltrilety commented Feb 9, 2018

@ltrilety Noted with the recent 1.5.5 release. Can you please file a GitHub issue for it? Thanks.

@julienlim Fine I created Tendrl/ui#812. I was not sure if that's an issue, as I don't much care if there's one or the other. However for sure there is some discrepancy between specification and the current state.

@ltrilety
Copy link

ltrilety commented Feb 9, 2018

@julienlim @gnehapk I noticed a small discrepancy between design - Concept B and this specification when import fails.

  • Design says that the modal window should not just display an error message, but instead it should display import task details. Moreover list of hosts should be available anytime. Though not the same list as when user goes to Hosts page, just name (hostname) and IP address should be listed there.
  • Specification clearly states that simple error should be showed without possibility to see hosts.

Please unify those.

@julienlim
Copy link
Member

@ltrilety @gnehapk @mcarrano

Good points @ltrilety. We're going to update the design, and the current thought is to add another column "Hosts" that show the # of hosts, and that would be hyperlinkable to show the list of hosts (hostname + IP address) in a modal at any time. We're going to rearrange the columns to be cluster name, cluster version, managed, hosts, volumes, alerts, volume profiling (for the managed) followed by button(s), and cluster name, cluster version, managed, hosts, message (for the unmanaged) followed by button(s).

@mcarrano will be posting the updates to the design.

@mcarrano
Copy link

I have updated the design per @julienlim 's comment and posted to InVision here: https://redhat.invisionapp.com/share/8QCOEVEY9#/244738628_Clusters

@ltrilety
Copy link

ltrilety commented Feb 14, 2018

@julienlim @mcarrano @gnehapk @a2batic the design change is fine and I agree with that. however it doesn't solve the discrepancy I mentioned at all.

  • Updated design still provides hosts via View Details link, while this specification says
    Cluster, Host, and Volume details are hyperlinked from the object name (and not from a “Details” button).
  • In case of import fail, specification says there should be
    error should be listed in a modal for a failed Import Cluster on clicking "View Details" link.
    whilst design states that whole task details should be provided, which makes sense as simple error message doesn't have to be enough for more info see import task fail not clear error message ui#813.

BTW current build satisfy specification, in other words there's no hosts listing in the View Detail modal window and simple error message is displayed in case of import fails.

@a2batic
Copy link
Member

a2batic commented Feb 14, 2018

@ltrilety @gnehapk

The design before concept B was implemented, had "Details" button, which has been replaced by a hyperlink as per the design, "View Details" hyperlink in design is for on-going or failed action.

@julienlim @mcarrano, how the import failure task details should be displayed in import failure modal ?

@julienlim
Copy link
Member

julienlim commented Feb 14, 2018

@a2batic @ltrilety @gnehapk @mcarrano

The original design was to have the import task fail task details in the Import Failure Modal, but since we ended up using the Task Details, we're going with that which still provides the full task details.

We'll remove https://redhat.invisionapp.com/share/8QCOEVEY9#/screens/244738627 to reduce confusion.

We're also making some further design updates based on the https://github.com/Tendrl/documentation/wiki/Architecture-Meeting-Notes for 13 Feb 2018.

@ltrilety
Copy link

The original design was to have the import task fail task details in the Import Failure Modal, but since we ended up using the Task Details, we're going with that which still provides the full task details.

@julienlim I created a new Tendrl/ui#821 for this, I am just hoping I understand your comment correctly.

@fbalak
Copy link

fbalak commented Feb 16, 2018

@gnehapk @julienlim @a2batic
I am redirected to Hosts page when I am on Volumes (Events, Tasks,...) page for some cluster and I change context to another cluster. Is this the expected behaviour? Shouldn't be user redirected to page relevant to the page that was selected before change of context? E.g. after change of context for Volumes page there might be rendered Volumes page for selected cluster (but I see the problem to render pages that are deeper in navigation like bricks for selected hosts or specific tasks).

@julienlim
Copy link
Member

@fbalak @gnehapk @a2batic @mcarrano

If you are on the Volumes (or any of the left-hand nav bar) for ClusterA, and switch context now to ClusterB, I would expect to see the landing page for ClusterB -- in our case, we don't have a landing page (dashboard) and the closest is the Hosts List.

Therefore no matter where you are in the left-hand nav (e.g. Volumes List, Brick Details, etc.), whenever you switch Clusters, you should expect to see the Hosts page.

Copy link
Contributor

@r0h4n r0h4n left a comment

Choose a reason for hiding this comment

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

@Tendrl/specs please approve individually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants