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 #14869 - Adds import button to environments welcome page #3501

Closed
wants to merge 1 commit into from

Conversation

shlomizadok
Copy link
Member

No description provided.

@@ -11,4 +11,8 @@
<div class="blank-slate-pf-main-action">
<%= new_link(_('New Puppet environment'), {}, { :class => "btn-lg" }) %>
</div>
<p><%= _('Or') %></p>
Copy link
Member

Choose a reason for hiding this comment

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

If there's no proxy I see 'Or' and nothing below that. It should check for at least one proxy to display this part. Otherwise looks good.

envs

Copy link
Member

Choose a reason for hiding this comment

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

That breaks https://www.patternfly.org/patterns/empty-state/, please try to adapt to use it @shlomizadok 😄

Copy link
Member

Choose a reason for hiding this comment

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

how strict should we follow it? according to the PF, it should be separate button for each proxy but I'd prefer having the select (current implementation) since there might be a lot of proxies available...

@shlomizadok
Copy link
Member Author

shlomizadok commented May 9, 2016

Removed the 'Or' sentence. Adapted to pf

@ares
Copy link
Member

ares commented May 9, 2016

Tested, works fine, @dLobatog any thoughts on the select vs. multiple buttons? If not, I'd merge.

@@ -11,4 +11,7 @@
<div class="blank-slate-pf-main-action">
<%= new_link(_('New Puppet environment'), {}, { :class => "btn-lg" }) %>
</div>
<div class="blank-slate-pf-secondary-action">
<%= select_action_button( _('Import'), {}, import_proxy_links(hash_for_import_environments_environments_path, 'btn btn-default')) %>
Copy link
Member

Choose a reason for hiding this comment

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

We already have a import_proxy_select helper that does this (and is used in the index page for the same button)

Copy link
Member Author

Choose a reason for hiding this comment

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

@tbrisker - You are right. Haven't used it in the first place because I have changed the css classes. But after @dLobatog's comment reverted back the css classes.
Anyhow, changed to import_proxy_select. Thanks

@shlomizadok
Copy link
Member Author

Fails on Ruby 2.3 unrelated(?) JS test

@ares
Copy link
Member

ares commented May 10, 2016

[test]

@ares
Copy link
Member

ares commented May 10, 2016

Merged as 8aea793, thanks @shlomizadok

@ares ares closed this May 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants