search

y7kim / agency-jekyll-theme

Agency Theme for Jekyll
Apache License 2.0
868 stars 2.02k forks source link

Recommend just using forloop.index instead of requiring a modal-id property to be set for every post. #71

Open georgeslabreche opened 6 years ago

georgeslabreche commented 6 years ago

Using {{forloop.index}} instead of {{post.modal-id}} here: https://github.com/y7kim/agency-jekyll-theme/blob/0a9be6a3ffea3d464b6f87c8197365c45acf830e/_includes/portfolio_grid.html#L13

and here: https://github.com/y7kim/agency-jekyll-theme/blob/0a9be6a3ffea3d464b6f87c8197365c45acf830e/_includes/modals.html#L3

Will eliminate the need for set and track modal-id properties in every post that is created.

georgeslabreche commented 6 years ago

Alternatively could just use {{ post.title | slugify }} instead of portfolioModal{{ post.modal-id }}, this would result in a SEO friendlier URL to share direct link to modal.

y7kim commented 6 years ago

Good idea, you're welcome to create a PR

On Tue, Apr 3, 2018, 6:05 PM Georges Labrèche notifications@github.com wrote:

Using {{forloop.index}} instead of {{post.modal-id}} here:

https://github.com/y7kim/agency-jekyll-theme/blob/0a9be6a3ffea3d464b6f87c8197365c45acf830e/_includes/portfolio_grid.html#L13

and here:

https://github.com/y7kim/agency-jekyll-theme/blob/0a9be6a3ffea3d464b6f87c8197365c45acf830e/_includes/modals.html#L3

Will eliminate the need for set and track modal-id properties in every post that is created.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/y7kim/agency-jekyll-theme/issues/71, or mute the thread https://github.com/notifications/unsubscribe-auth/ADtkiIjJNmsthxhG6nYjOWnKYGQwHJNuks5tlBxUgaJpZM4TGAzS .