Removing the listnav from Physical Infra Manager page

[douglasgabriel]

This PR is able to

• Add an instance variable just to remove the listnav;
• Remove the listnav on Physical Infra Manager page.

Current result
Once that Physical Infra Manager page doesn't have any filter, the listnav always shows a pointless alert.

After this PR
The space is better used without the listnav

[douglasgabriel]

@miq-bot assign @AparnaKarve
I'm not sure if the travis failure is due to my changes here 🤔
And seems like the codeclimate is complaining about something that already exists.

[douglasgabriel]

@miq-bot add_label compute/physical infrastructure

[douglasgabriel]

[UX Review] @terezanovotna, could you take a look on this, please?

[miq-bot]

Checked commit douglasgabriel@ce93e8e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

[terezanovotna]

@miq-bot add-label ux/review

[douglasgabriel]

@miq-bot add_label ux/review

[skateman]

@miq-bot add_reviewer @martinpovolny

[skateman]

I am pretty much afraid this is not The Right Way™ 😞, the layout_uses_listnav? is conceptually bad and increasing its complexity is definitely not something we want.

[douglasgabriel]

I see your concern, but I have to disagree about "increasing its complexity", IMHO I'm adding a new option of layout with a simple and semantic variable, where, if a page doesn't need the listnav, remove it is simple like write @without_listnav = true. Anyway, if there is a better way to do this, please, let me know

[skateman]

I heard some rumors that @himdel was dealing with the same issue in v2v, so summoning him 😉 🍷

[himdel]

Right,
I think the problem with @without_listnav is that it's not saying "I want such and such layout", it's only saying "don't go listnav, use whatever else".

You could still probably use the center_div set of layouts, but that still involves touching all those helpers.

Or you can use the new method which pretty much involves saying "this is my layout, use it" :)

So, the PR was #3970
Essentially, you can do (in your controller):

helper do 
  def layout_full_center
    "layouts/my_custom_layout"
  end
end

and create app/views/layouts/_my_custom_layout.html.haml.

(Obviously your helper may need to decide per-screen.)

EDIT: you also need inner_layout_present? to return false, but that should be trivial

[martinpovolny]

I am all for moving this decision logic to the active controller and using the ugly conditions and lists of screens only as a fallback. So I 👍 what @himdel wrote.

Thx!

[mzazrivec]

I don't quite understand why we want to get rid of the listnav layout here in the first place.

If there are filters defined, the left side would show filters, without filters only a flash message will be rendered. This is how it works everywhere (not just on this one screen) and I don't think we can introduce this kind of inconsistency and diverge from the rest of the application.

If the above behavior and appearance is not wanted, then we need to change it everywhere and consistently.

Until then all the above discussions are moot, at the very least.

[himdel]

Agreed, there are no other screeens doing this, phinfra should not be special.

Maybe we could consider turning the listnav into a proper sidebar - one that can be hidden, or expaned (floating/pinnable?) on demand. But if so, that should happen everywhere, not just this one screen.

[skateman]

@himdel we do have this feature on explorer screens, so not impossible 😉

[mzazrivec]

Also, it seems the search div is gone with this change.

[terezanovotna]

The listnav is really helpful in terms of navigating in the product. See here:

and thanks @himdel for the explanation.

I agree @douglasgabriel it's redundant to see the listnav filter when no filter is applied. How about making it collapsible?

I'm checking with the bigger UXD/PatternFly team to discover if this type of collapsible is used elsewhere

[douglasgabriel]

I have no doubts about the importance of the listnav, just agree with the a tip from @Rohoover to enjoy better the content space removing a unused component (at the moment) from the user view. But I'm okay to follow what you guys decide 😄

[himdel]

Also, please note that the listnav is not empty for physical infra either.

The moment a user saves a filter, you get this:

It's just that we have no default global filters on that screen.

[douglasgabriel]

Hmm, got it @himdel . Those filters are not be shown to me, in this case there is no reason to remove the listnav, sorry about misunderstanding the listnav operation. If you agree, I can close this PR, then

[himdel]

Up to you :)

I do think that any solution would need to work for all the screens, not just this one.

Whether finding a "nicer" solution is a priority or not, I can't say..

[terezanovotna]

@douglasgabriel can you add a little description why it was closed? (just making sure the issue is clear)