-
Notifications
You must be signed in to change notification settings - Fork 358
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
About modal: render pretty looking plugin names #4496
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miq-bot add_label formatting/styling |
Are you guys also OK with the plugin names as I proposed them above? ( |
Good idea IMO :) One possible nit-pick... in a section called Plugins, maybe we should not repeat "* Plugin" on every line. |
2224132
to
a69ac95
Compare
Checked commit mzazrivec@a69ac95 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
If we're showing low-level "master@1c00fee" in the dialog (which I think is good & valuable), perhaps it'd more useful (even if less "pretty") to use actual repo names, e.g. "manageiq-automation_engine master@1c00fee" ? EDIT: not sure if repo name or gem name is most useful, I think in most cases they're same |
That could be useful. What about both? Show the pretty name in the list, and add a "raw" |
Ditto the repo name sentiment, goal of this information is to be handy, to facilitate quick accurate identification which version of what code is employed in a given instance... Pretty things are nice.. but forcing me to look for what Edit: but this is one hellofuh awesome body ah work, no two ways about it, well done! 🤗 🙇♀️ |
Seems like there is still some unfinished work from #4423 as the "table" (divs with spacing) view is not done yet.... Any chance we could make all that info fit?
Also.. as for the raw info... seems like "ManageIQ::Providers::Whatever" is still essentially useless, and the raw text displayed should be |
My intent here (and in the PRs linked) is only to add the display names for plugins. The styling work will / should be done in separate PRs. |
But we don't really know what the semantics of the string
Right, but like I wrote above, |
Sure, I was not suggesting changing the styling here, just saying that if the plan is to have something more table-like, we may get away with the 3 fields instead of my previous idea of using a title. Whether any of that is a good idea or not... ¯\_(ツ)_/¯ :) |
Well, the one thing that goes for the second string IMO is that it's the repository name. That said, it's not something that we can make match in 100% cases so... that may break too.. |
@himdel As I mentioned in #4423, my plan is/was.. to use a unstyled list like the one at the top of the modal - not a table or something table-like. If that won't work, we'll need to loop UX back in. |
OK, so.. looks like we can merge and figure out the rest elsewhere, if needed :). |
I'm proposing to extend the nice work from #4423 with presentable - looking plugin names.
Before:
After:
The plugin names with my changes in place:
Plugin PRs:
ManageIQ/manageiq-v2v#603
ManageIQ/manageiq-api#457
ManageIQ/manageiq-automation_engine#214
ManageIQ/manageiq-consumption#140
ManageIQ/manageiq-content#411
ManageIQ/manageiq-graphql#60
ManageIQ/manageiq-providers-ansible_tower#119
ManageIQ/manageiq-providers-amazon#477
ManageIQ/manageiq-providers-azure#283
ManageIQ/manageiq-providers-foreman#22
ManageIQ/manageiq-providers-google#68
ManageIQ/manageiq-providers-kubernetes#277
ManageIQ/manageiq-providers-kubevirt#113
ManageIQ/manageiq-providers-lenovo#221
ManageIQ/manageiq-providers-nuage#135
ManageIQ/manageiq-providers-openshift#107
ManageIQ/manageiq-providers-openstack#336
ManageIQ/manageiq-providers-redfish#23
ManageIQ/manageiq-providers-scvmm#83
ManageIQ/manageiq-providers-vmware#312
ManageIQ/manageiq-providers-ovirt#279
ManageIQ/manageiq-schema#261
@Fryguy @epwinchell @skateman @terezanovotna