Manageiq-ui-classic: History toolbar using old breadcrumbs

Created on 29 Jul 2019  路  14Comments  路  Source: ManageIQ/manageiq-ui-classic

The history toolbar is still using @breadcrumbs, even though breadcrumbs aren't using it since https://github.com/ManageIQ/manageiq-ui-classic/pull/4468.

So:
we should change the history toolbar to either use history, or use the new breadcrumbs.
And we can remove all the @breadcrumbs logic after that. :)

Cc @rvsia @martinpovolny

help wanted technical debt

Most helpful comment

@h-kataria @martinpovolny @himdel @terezanovotna @epwinchell I will prefer to focus on breadcrumbs and not have mutliple "back" user experience

All 14 comments

We need to check if we can actually remove the History toolbar. With @epwinchell and Loic and @terezanovotna .

can you elaborate more on history toolbar @himdel? not sure what you mean

Sorry, sure :)

history

It's the green thing in the picture - basically a toolbar we have on some screens, with or without the refresh button, that allows you to go back in history.

Expanded:

histroy2


Before the new breadcrumbs, the content of the history toolbar would match the old breadcrumbs (and the breadcrumbs would not be present on the screen, I think).

Now, the new breadcrumbs are everywhere, and using a different structure (relationship) than the old ones and the toolbar (history).

I can imagine that the breadcrumbs are not perfect everywhere. What I suggest is that moving forward we concentrate on making sure that the breadcrumbs are (almost) perfect and start removing the history toolbar.

@martinpovolny I've been digging around and can't seem to find exactly why this was added in the first place. I think it started with an RFE re the browser back button blowing up. I'm for ditching it, we need the real estate.

I would like to know whether people actually find value in the history back button.

We know the implementation of breadcrumbs was a success - and I can imagine this toolbar back button is another way how to browse hierarchy. I am ok with dropping the history functionality - this is not even known UX pattern. And use breadcrumbs and toolbar back button to navigate through breadcrumbs.

Thoughts @loicavenel ?

@h-kataria this would be a good topic to discuss on the UI review.

@martinpovolny @epwinchell @himdel history toolbar was added in explorer type screens as an alternative to breadcrumbs and also to navigate back to last number of items visited in each accordion in the explorer. Let's discuss on the UI review call to see if anyone really uses this, i haven't seen many BZs open in that area so i am assuming not many users are using that.

@h-kataria @martinpovolny @himdel @terezanovotna @epwinchell I will prefer to focus on breadcrumbs and not have mutliple "back" user experience

@bmclaughlin please feel free to ping @rvsia @himdel if you have questions on this one.

Thanks @Loicavenel.

If I understand it, we are emphasizing breadcrumbs and wherever we have this Back button, we either get rid of it everywhere, or use it as going back (in a hierarchical way).

With #6050 merged, is there more work remaining on this issue?

Remaining work is this:

And we can remove all the @breadcrumbs logic after that. :)

lucifer - [~/Projects/manageiq-ui-classic] (master)$ grep -r @breadcrumbs app/ | wc -l
99

Carefully remove all of the above. With special attention being payed to the ones that look like this one:

app/controllers/application_controller/miq_request_methods.rb:      javascript_redirect(@breadcrumbs.last[:url])

High probability of breaking something somewhere w/o noticing it unless a lot of :eyes: are applied to the code and a lot of :computer_mouse: clicking is applied to the UI when testing.

Do not rush with this one, whoever will find the time to finish it, please.

@breadcrumbs is still being used to determine title on the screen via drop_breadcrumb method in various places, i will take a stab at cleaning this up.

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

Was this page helpful?
0 / 5 - 0 ratings