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

Use the same sidebar logic that Solidus uses #52

Merged
merged 1 commit into from
Nov 8, 2019
Merged

Conversation

elia
Copy link
Member

@elia elia commented Oct 25, 2019

This PR was extracted from #6

This change makes the default static_content sidebar use the same logic
that Solidus does in frontend/app/views/spree/products/index.html.erb.

The positive consequence of this change is that now, by default, static
content pages render the same default sidebar that Solidus pages do,
unifying the look and feel and making the static content pages look
"normal".

Fixes #6
Closes #16

@elia elia self-assigned this Oct 25, 2019
This change makes the default static_content sidebar use the same logic
that Solidus does in frontend/app/views/spree/products/index.html.erb.

The positive consequence of this change is that now, by default, static
content pages render the same default sidebar that Solidus pages do,
unifying the look and feel and making the static content pages look
"normal".
Copy link
Member

@aldesantis aldesantis left a comment

Choose a reason for hiding this comment

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

Approved by mistake. :(

@elia elia requested a review from aldesantis November 8, 2019 14:09
Copy link
Member

@aldesantis aldesantis left a comment

Choose a reason for hiding this comment

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

I think this makes sense for now to achieve consistency, although there's definitely room for future improvement.

@elia elia merged commit 9ed3cf5 into master Nov 8, 2019
@elia elia deleted the elia/pr-6-reprise branch November 8, 2019 16:14
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.

None yet

3 participants