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

Added a link to the frontend product from the backend product edit page #2956

Merged

Conversation

seand7565
Copy link
Contributor

It's handy to be able to quickly get to the frontend product page from the backend product edit page.

@jacobherrington
Copy link
Contributor

jacobherrington commented Nov 15, 2018

Could we add some margin between the buttons?

image

Adding <%= link_to t('spree.new_product'), new_object_url, id: 'admin_new_product', class: 'btn btn-primary mr-2' %> makes it a bit better, don't you think?

image

@jacobherrington
Copy link
Contributor

jacobherrington commented Nov 15, 2018

I like usability ideas like this one.

There is also a danger that we are assuming the individual is using both solidus_frontend and solidus_backend. I think a fair number of people choose to forgo solidus_frontend.

@seand7565
Copy link
Contributor Author

@jacobherrington Very good point about people not using frontend. Any suggestions on how to handle that? The admin navigation footer wraps the link to the frontend in if spree.respond_to? :root_path - a similar approach should work here, right?

@jacobherrington
Copy link
Contributor

@seand7565 That is something I had thought of as well, the question is: Do we want to perpetuate that pattern? I think that's a call for the core team.

@tvdeyen
Copy link
Member

tvdeyen commented Nov 16, 2018

I like the idea, but we need to address stores not having the frontend installed. solidus_support uses defined?(Spree::Frontend::Engine)

@seand7565 seand7565 force-pushed the add_admin_product_to_frontend_button branch from 2179d67 to 9e7e83e Compare November 17, 2018 04:19
@seand7565
Copy link
Contributor Author

@tvdeyen I like defined?(Spree::Frontend::Engine) - I've popped that in, as well as a left margin on the new button so they don't look so squashed together.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Nice 👍

@kennyadsl
Copy link
Member

kennyadsl commented Nov 20, 2018

I like this but maybe it's time to port these methods into solidus_core?

@tvdeyen
Copy link
Member

tvdeyen commented Nov 20, 2018 via email

@kennyadsl
Copy link
Member

Or add solidus_support as runtime dependency?

I don't think this could work as is since solidus_support already depends on solidus_core.

@tvdeyen
Copy link
Member

tvdeyen commented Nov 20, 2018

I don't think this could work as is since solidus_support already depends on solidus_core.

Mmm, right. Is this actually necessary? We could assume that extensions or shops that install solidus_support also will have solidus_core installed, no?

@jacobherrington
Copy link
Contributor

We could assume that extensions or shops that install solidus_support also will have solidus_core installed, no?

@tvdeyen Yes. solidus_support depends on solidus_core so a site using solidus_support without solidus_core wouldn't work anyway.

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

Love this!

@kennyadsl kennyadsl merged commit e3caf3a into solidusio:master Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants