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

Fixes #24091 - add pf empty state design to hostsgroup #5743

Merged
merged 1 commit into from Aug 22, 2019

Conversation

amirfefer
Copy link
Member

before:
hostsgroup1

after:
hostsgroup2

@theforeman-bot
Copy link
Member

Issues: #23920

boaz0
boaz0 previously approved these changes Jun 26, 2018
Copy link
Member

@boaz0 boaz0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amirfefer
Copy link
Member Author

amirfefer commented Jun 26, 2018

I've noticed the same issue with media welcome page, @ares did you notice it in other pages?
This probably won't be cherry picked into 1.18 due to text changes (@ohadlevy ?)
However I found a regression in application_content layout and it could be fixed for 1.18 with no pf redesign, still I think it would be great to move these welcome pages (media, hostgroup) to the patternfly empty state design on 1.19.

Edit: a regression fix opened here #5748

</div>
<h1><%= _('Host Groups') %></h1>
<p><%= _("Host Groups allow hosts with common configuration to be defined and grouped together.
in some ways similar to an inherited node declaration, in that it is a high level grouping of classes that can be named and treated as a unit.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence doesn't make much sense like it is, did something get removed by mistake? cc @GregSutcliffe if you can think of a good 'welcome' message for host groups that'd be helpful 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion: Hosts can be grouped together to share common configuration. Nested groups will inherit from their parent(s).

@amirfefer amirfefer changed the title Fixes #23920 - add pf empty state design to hostsgroup Fixes #24091 - add pf empty state design to hostsgroup Jun 27, 2018
@ohadlevy
Copy link
Member

whats the status of this PR? thanks :)

@ohadlevy
Copy link
Member

@amirfefer bump??

@amirfefer
Copy link
Member Author

rebased + updated
I've used @ekohl suggestion,

<div class="blank-slate-pf-main-action">
<%= new_link(_("Create Host Group"), :class => 'btn-lg') %>
</div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: missing newline

</div>
<h1><%= _('Host Groups') %></h1>
<p><%= _("Host Groups allow hosts with common configuration to be defined and grouped together.
Hosts can be grouped together to share common configuration. Nested groups will inherit from their parent(s).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Host Groups allow hosts with common configuration to be defined and grouped together. Hosts can be grouped together to share common configuration.

These 2 sentences are almost identical. I would keep just one of them.

@xprazak2
Copy link
Contributor

@amirfefer, any progress on this?

@tbrisker
Copy link
Member

ping @amirfefer, this one looks almost ready to merge and just needs a minor change and rebase, do you want to do that or shall we close it?

@amirfefer
Copy link
Member Author

@tbrisker Thanks for the reminder, done :)

Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amirfefer !

@tbrisker tbrisker merged commit 988546c into theforeman:develop Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants