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

History toolbar using old breadcrumbs #5892

Open
himdel opened this issue Jul 29, 2019 · 14 comments
Open

History toolbar using old breadcrumbs #5892

himdel opened this issue Jul 29, 2019 · 14 comments

Comments

@himdel
Copy link
Contributor

himdel commented Jul 29, 2019

The history toolbar is still using @breadcrumbs, even though breadcrumbs aren't using it since #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

@martinpovolny
Copy link

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

@terezanovotna
Copy link

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

@himdel
Copy link
Contributor Author

himdel commented Jul 30, 2019

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).

@martinpovolny
Copy link

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.

@epwinchell
Copy link
Contributor

epwinchell commented Jul 31, 2019

@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.

@terezanovotna
Copy link

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.

@h-kataria
Copy link
Contributor

@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.

@Loicavenel
Copy link

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

@h-kataria
Copy link
Contributor

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

@terezanovotna
Copy link

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).

@chessbyte
Copy link
Member

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

@martinpovolny
Copy link

martinpovolny commented Oct 16, 2019

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 👀 are applied to the code and a lot of 🖱️ clicking is applied to the UI when testing.

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

@h-kataria
Copy link
Contributor

@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.

@miq-bot
Copy link
Member

miq-bot commented Jun 11, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
General revamp
  
Awaiting triage
Development

No branches or pull requests