Display Plugin commit hashes in the About Modal #4423
Conversation
|
@miq-bot add_label ux/review |
|
@epwinchell Can you check the css part for the scrollbar? |
|
Checked commits AparnaKarve/manageiq-ui-classic@68d530f~...646e0fd with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 **
|
@terezanovotna Any issues with presenting hashes via a scrollbar in the modal? |
|
@epwinchell @terezanovotna Just to give you an idea, without the scrollbar, it would look like this -
|
|
Hi @AparnaKarve I agree it makes sense to hide the Pluggins and show them on click. How about using this Expand-Collapse pattern?. (search for Expand-Collapse Section > Launch demo modal) "Advanced Options" would be replaced with "Plugins".
|
| def vmdb_plugins_sha | ||
| Vmdb::Plugins.versions | ||
| end | ||
|
|
There was a problem hiding this comment.
Both methods shrunk into one:
def vmdb_plugins
Vmdb::Plugin.versions.collect do |engine, sha|
"#{engine.gsub(/ManageIQ::|::Engine/, '')} #{sha}"
end
end
| - vmdb_plugins_sha.each do |engine, sha| | ||
| %div | ||
| = "#{plugin_name(engine)} #{sha}" | ||
|
|
There was a problem hiding this comment.
... and then in view:
- vmdb_plugins.each do |plugin|
%div
= plugin
|
I'm also wondering, what the information value in a string like |
|
@mzazrivec : well that's what the RFE asked for. The commit hashes will come in handy when dealing with issues in the production. |
I'm not questioning the commit hashes, but the |
@terezanovotna The Expand-Collapse styling does not quite address the problem, since it looks like we would still need the scrollbar.
The main challenge here is we are trying to fit a really long list of items inside a modal and not a form. |
|
@terezanovotna @epwinchell So we have 2 options -
or
|
|
@AparnaKarve still not a big fan of the scrollbar - it scrolls over two lines and therefore is not usable. I like the look of the collapse/expand! I think it's ok that the modal gets bigger when expanded. If that becomes a super long list, the user has to scroll down, but that's a normal behavior. I vote for this option one. |
Please clarify what you mean by it's not usable, because I can scroll up and down the list with no issues.
If this were a form, I would have completely agreed with you -- but in the context of the Modal, this just does not apply... and especially the About modal (the modal that represents the CloudForms product) Current issues with expand/collapse -
A picture is worth a thousand words and a video is worth a thousand pictures, so here's a video - |
|
@terezanovotna @AparnaKarve I think you're both right and wonder if a compromise approach is better I agree with @terezanovotna that the twisty is nicer. One thing I love about the twisty is that in the future if we need to show other information (like, say, a long list of licenses or something), we can just put that in a second twisty, so it lets us conserve the vertical real estate for other details. However, as you can see, we already have a gazillion plugins so it blows out the vertical size anyway once you twist it open. So, on that side, I agree with @AparnaKarve that a scrollbar allows you to constrain the vertical size, but 2 rows is just too few. So, how about use the twisty to expand/collapse, but inside the expanded section have a scrolling section that's, say, 10 or 15 items high? |
The original code showed
but a) the I like the idea of having a pretty string, but where would that live? A |
|
One minor thing...I don't really like how the version is right up against the plugin name. It's hard to visually separate the two things...perhaps an inline table might work better (doesn't need a header, I just put one for this example below)
|
@Fryguy Agreed - just to keep things simple here, I think we should handle pretty strings separately. For now, I am just going to continue iterating on some of your suggestions above. |
|
@Fryguy Well done on compromise! If we apply scroll over 10-15 lines, I am good with that solution! @AparnaKarve Could we apply a scroll that looks more PF? Here is an example I found, different context, but shows the look of a scroll bar. |
|
@terezanovotna Based on @Fryguy 's suggestions above, the plugin info is now being displayed in a table. I think the modal is looking much more presentable with the table. Video: https://drive.google.com/open?id=1-hkc3RhFUsdSJ2CE7EFRKa3ZBBfRnQpf Let me know what you think. |
|
From the usability standpoint, LGTM! From the visual perspective, we are introducing a new look (the visual appearance of this table is not PF pattern). Can we at least hide the white border of the table? and still have the scrollbar on the side? Right now, it's not pretty. But it's your call if we want pretty UI or not. PatternFly goal is to achieve a consistent look and feel across Red Hat product portfolio, so we can have user friendly products. We all can contribute to this design system library and then we apply these patterns in our products. My aim is to find a balance between technical feasibility and usability, so we can deliver user friendly product to our end audience. PF are basically design rules we apply to the product, and I only provide suggestions. |
|
With all respect I don't like where this is going. I am not an UX expert. Also I don't care much as this feature is really well hidden in the product and only people looking for the version will see it and will find what they need there. But I'd like not to erode the process here. We have developers here and we have designers. There's a process that includes the UX review and has some rules. Mergers formally signed a paper requiring them to respect that process. My feeling from this discussion is that @terezanovotna is backing off and ux/approving something that she really does not like (and I can see why) under a pressure and out of fear that there might be unpleasant feelings and a conflict. I would like to get this back to the factual level. So I have only two questions here and those are for @terezanovotna :
If the answer is "Yes" to any of the 2, then I am ok with this. If the answer is "No" to both of them, then this should not be ux/approved. |
|
|
@martinpovolny So you don't think developers should question UX when they think something is wrong...particularly on something that is "new"? Put another way, we discussed, compromised, and gave the final rubber stamp to UX. I'm not sure I understand your objection. We can take off-PR if you want 😄 |
No. I think that the UX people should not back off in situations when they are pretty sure that what they are saying is correct just to avoid a potential conflict. And not only is that not good for the project, but it also does not match the UI/UX review rules that we signed. I am basically asking @terezanovotna to carry on doing what she is doing and what worked in other PRs. I'll accept your suggestion: If you want to dig deeper into this, we should talk f2f. |
On the contrary, I think @terezanovotna has given her most candid opinion -- I like how she has handled the situation. Also bear in mind that the nice, clean PF tables that we all love to look at, have a fresh white background and plenty of real estate space. I'm no designer either, so the way I'm designing this is by taking input from people while taking into account the Space and Color constraints. I have tried various permutations and combinations that I could possibly think of (border, no border etc) and I sincerely feel that the look-and-feel that we have right now in the video is by far the best.
The border, IMO, adds character to the table and is also more eye-friendly especially during scrolling. Ok, enough said... I really like this feature considering how much back-and-forth we have done in the past with QE when v2v was in it's prime testing phase, and I wish we had this sooner. |
|
I like the implementation you guys have done on the modal. The additional chevron that hides the plugins is a nice touch for users not interested in the information. As to table or not to table, we are arguing over a visual preference that has no standard currently dictated by PF. PF has tables both with lines or no lines so that is not a good indicator on which is more "correct". Since the modal has it's own distinct visual identity, I don't think the standard table would be appropriate here. For historical knowledge, when I designed the About Modal 1.5 years ago we had a conversation about how we handle products that have very large "about" sections but there hasn't been any movement there. We can also check-in with other products to see how this was handled if anyone is interested. It would take a few days to compile that info. |
I find the border, the vertical & horizontal table lines irritating. I understand Jason's table suggestion more like: the plugin name & plugin version should be nicely aligned to make the whole dialog more readable. Not that it necessarily has to be an actual table with lines and borders. Above, you said yourself that in the modal dialog we're limited by space constraints, so I'd rather for us not to add any more visual noise than it's necessary. Besides, in the modal part that renders basic application info (version & stuff) we don't use any borders and lines either. Lines & borders in the plugin part would just add visual inconsistency (IMO). |
I think if the table is replaced with the same unstyled list that @AparnaKarve showed in the original screenshot, but with a couple pixels of padding between engine and sha, we'll be all set - clean & simple. |
|
Merging, I think the best suggestions have been implemented and the current version is quite usable. @epwinchell can follow up with style tweaks. |
Depends on the work done in PR ManageIQ/manageiq#17755
ManageIQ/manageiq-v2v#422
https://bugzilla.redhat.com/show_bug.cgi?id=1594348