theforeman / foreman_puppet

https://theforeman.org/plugins/foreman_puppet/
GNU General Public License v3.0
7 stars 24 forks source link

Puppet detail box #293

Open ekohl opened 2 years ago

ekohl commented 2 years ago

On the new host detail page there is on Puppet -> Reports a detail box titled Puppet details.

I'm a bit surprised by its placement on the Reports page because in my mind the information doesn't really correlate.

For example, the details are what should be applied, but this may or may not match reports. It may even have changed somewhere between the reports. Shouldn't it be its own tab called Summary or similar?

Then we have what's on the panel itself.

This is a link to the Puppet environment's edit page. Is that really the expected page? I'd be worried that a user thinks they can edit the environment that the host belongs to while in fact it changes the environment name for a lot of hosts. The old detail page links to all hosts within that environment and I think makes more sense.

We always capitalize Smart Proxy. It can also be a link to the Smart Proxy's detail page (and possibly preselect the Puppet tab).

The name doesn't feel very consistent to me. I'd name it Puppet CA Smart Proxy because it is the Smart Proxy with the Puppet CA feature. Here you can also link in the same way as Puppet Smart Proxy.

This is with foreman_puppet 4.0.1 on Foreman 3.3.0.

Ron-Lavi commented 2 years ago

Hey @ekohl thanks for the input!

For example, the details are what should be applied, but this may or may not match reports. It may even have changed somewhere between the reports. Shouldn't it be its own tab called Summary or similar?

We didn't want to create an additional tab for just 4 rows, is there any other tab that makes more sense? or more content that should be added so it makes sense to create a new tab for? here are some of the original mocks: https://marvelapp.com/prototype/d45i26d/screen/85913489

Puppet environment This is a link to the Puppet environment's edit page. Is that really the expected page? I'd be worried that a user thinks they can edit the environment that the host belongs to while in fact it changes the environment name for a lot of hosts. The old detail page links to all hosts within that environment and I think makes more sense.

does it make sense if we link to the environment index page with a search query to the specific host? e.g: /foreman_puppet/environments?search=name+%3D+{HOSTNAME}

Puppet smart proxy We always capitalize Smart Proxy. It can also be a link to the Smart Proxy's detail page (and possibly preselect the Puppet tab).

As for capitalization, seems that Patternfly ux-writing guidelines are pushing for a lower-case usage, though in our docs I found it's capitalized, e.g: https://docs.theforeman.org/nightly/Provisioning_Guide/index-foreman-el.html any other thoughts @marisvirik @Manisha15 ?

I like the idea of preselecting the puppet tab on the smart proxy's detail page.

Puppet server CA The name doesn't feel very consistent to me. I'd name it Puppet CA Smart Proxy because it is the Smart Proxy with the Puppet CA feature. Here you can also link in the same way as Puppet Smart Proxy.

Sounds good 👍🏼

ekohl commented 2 years ago

does it make sense if we link to the environment index page with a search query to the specific host? e.g: /foreman_puppet/environments?search=name+%3D+{HOSTNAME}

That does not make sense since there is no environment with a specific hostname. You would get a table with "No entries found" as the body.

As for capitalization, seems that Patternfly ux-writing guidelines are pushing for a lower-case usage, though in our docs I found it's capitalized, e.g: https://docs.theforeman.org/nightly/Provisioning_Guide/index-foreman-el.html any other thoughts @MariSvirik @Manisha15 ?

I think we should treat Smart Proxy as a name, just like John. This indicates that Smart Proxy is a concept. If you'd have a title that's Hello {name}, you'd use Hello John and not Hello john.

Ron-Lavi commented 2 years ago

does it make sense if we link to the environment index page with a search query to the specific host? e.g: /foreman_puppet/environments?search=name+%3D+{HOSTNAME}

That does not make sense since there is no environment with a specific hostname. You would get a table with "No entries found" as the body.

Sorry I meant /foreman_puppet/environments?search=name+%3D+{environment_name}

does it make more sense?

ekohl commented 2 years ago

I thought a bit about and I think it'd be an improvement over the current solution. At least that table has a link to the hosts and a link to import classes (in the actions). IMHO what you really want is a proper detail page for a Puppet environment but currently doesn't exist. Linking to that table is the next best thing.

How about we follow your suggestion and link to the overview while we also create an issue for a new Puppet environment detail page? I can do the latter.

Ron-Lavi commented 2 years ago

Sounds good, thanks @ekohl :)

ekohl commented 2 years ago

https://github.com/theforeman/foreman_puppet/issues/299

MariSvirik commented 2 years ago

@ekohl absence of detail page is exactly what we talked about yesterday with Manisha and Ron.

Smart proxy issue: It's common that the docs and PF4 have different capitalization (of course not an ideal state). I hear your reasoning, on the other hand, it does not fall under any PF4 exception for using the title case (I checked that with the PF4 team to be perfectly sure) Unfortunately, in UI we should always follow PF4 standards - so sentence case it is. Feel free to raise an issue with the Patternfly team.

Manisha15 commented 2 years ago

As for capitalization, seems that Patternfly ux-writing guidelines are pushing for a lower-case usage, though in our docs I found it's capitalized, e.g: https://docs.theforeman.org/nightly/Provisioning_Guide/index-foreman-el.html any other thoughts @MariSvirik @Manisha15 ?

Yes, Based on documentation and other Smart Proxy examples we can keep it capitalized, i.e, Puppet Smart Proxy.

MariSvirik commented 2 years ago

Thanks to this discussion I contacted the documentation team, and we scheduled a meeting for Monday. But from what they told me: they are following UI and not other way around. Our holy source of truth if we are using the PF4 design system for components and so on should be....PF4 guidelines. So sentence case it is.

Ping me if you are interested in joining the call on Monday with Vendula from the doc team. @Manisha15 @ekohl

UPDATE: I talked to doc team and they confirmed that documentation should and will change according to the UI which should follow PF4 standards ---> therefore Puppet smart proxy should be used here.

ekohl commented 2 years ago

I still think you're missing my point. IMHO we should have titles like Manage Foreman since Foreman is a name, but likewise Manage Smart Proxy because it is also a name. This doesn't have anything to do with titles: in regular text you should do the same.

So the discussion should be: do we treat Smart Proxy as a name/concept and always capitalize it or not. PF4's title guidelines are irrelevant in the discussion.

Manisha15 commented 2 years ago

I agree with @ekohl , since Smart Proxy is noun like Foreman or Katello so it should be Capitalized.

vferschm commented 2 years ago

Hi @ekohl @Manisha15 @MariSvirik I still think you're missing my point. IMHO we should have titles like Manage Foreman since Foreman is a name, but likewise Manage Smart Proxy because it is also a name. This doesn't have anything to do with titles: in regular text you should do the same.

I don't think so. Foreman= John, that's true. Smart proxy is not equal to John, because multiple companies use smart proxy as a feature and all of them have pretty similar functionality, therefore, it has become general noun. Does that make sense? Similarly, we use "next-gen firewall" (which means that you can block Instagram or LinkedIN and you don't have to block particular ports or that it will be autoconfigured when you plug it in your network) or "unified thread management" - when the first company came with this name, it was capitalized, but these days it is an only general noun without capitalization because many devices or software solutions use it and everyone knows the meaning of it.

ekohl commented 2 years ago

Smart proxy is not equal to John, because multiple companies use smart proxy as a feature and all of them have pretty similar functionality, therefore, it has become general noun.

I think this is false. Smart Proxy is a Foreman specific thing. It is not a general thing like HTTP proxies or otherwise. That is precisely my point. It may not be the best name (downstream's Capsule may be better), but it is what we've chosen.

All the examples when I search for smart proxy in a search engine are conceptually a different thing. They solve completely different problems are are IMHO not comparable.

Ron-Lavi commented 2 years ago

I think @ekohl is correct, I found this video about getting started with Foreman: https://youtu.be/NOAnaEipX8I where they mention how Smart Proxy naming is confusing 🤣 and that it should've been Foreman Proxy which explains well what it does.

I think that if it was called Foreman Proxy we would keep it uppercase, therefore it kinda makes sense for me to keep it Smart Proxy and not smart proxy.

I would suggest creating a poll in community.theforeman.org and asking about the two options.

MariSvirik commented 2 years ago

Alright, we wasted enough of our time here (involving at least four teams) ;D Let's keep it Puppet CA Smart Proxy and Smart Proxy.

ares commented 2 years ago

I wish we called Smart Proxy "Backman", it causes so many confusion even after those years :-)