Skip to content
This repository has been archived by the owner on Jun 21, 2018. It is now read-only.

Add page number to breadcrumb #232

Merged
merged 6 commits into from
Feb 12, 2016
Merged

Add page number to breadcrumb #232

merged 6 commits into from
Feb 12, 2016

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Feb 9, 2016

  • welcome#index
    • category#show
    • post#show

@mtsmfm mtsmfm deployed to daimon-news-multi-tenan-pr-232 February 9, 2016 07:00 Active
@@ -1,5 +1,6 @@
- content_for(:rel_next_prev_link_tags) { rel_next_prev_link_tags(@posts) }
- breadcrumb :category, @category
- breadcrumb :category_page, @category if params[:page]
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to use params on only controller.
@posts has current_page method, so why don't you use @posts.current_page instead of params?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know that @posts has such method 😢 .
Could you use @posts.current_page instead of params[:page]?

Copy link
Contributor

Choose a reason for hiding this comment

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

👌

@mtsmfm mtsmfm force-pushed the add-page-number-to-breadcrumb branch from aeb8ef4 to f40d1c6 Compare February 12, 2016 02:55
@mtsmfm mtsmfm deployed to daimon-news-multi-tenan-pr-232 February 12, 2016 02:55 Active
@okkez
Copy link
Contributor Author

okkez commented Feb 12, 2016

👍

mtsmfm added a commit that referenced this pull request Feb 12, 2016
@mtsmfm mtsmfm merged commit 8c20e4f into master Feb 12, 2016
@mtsmfm mtsmfm deleted the add-page-number-to-breadcrumb branch February 12, 2016 04:51
@mtsmfm mtsmfm removed the review label Feb 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants