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

pagy calendar #2

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

pagy calendar #2

wants to merge 6 commits into from

Conversation

yshmarov
Copy link
Member

@yshmarov yshmarov commented Jan 6, 2024

pagy-calendar

format.html do
flash[:notice] = "Event was successfully created."
redirect_to event_url(@event)
# redirect_to events_path(pagy_calendar_url_at(@calendar, @event.start_date))
Copy link
Member Author

Choose a reason for hiding this comment

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

i think that to make this redirect line work, we need to define @calendar and all the collection here.

if the url params were fixed

# bad
http://localhost:3000/events?year_page=10&month_page=10&day_page=5
# good
http://localhost:3000/events?year_page=2023&month_page=10&day_page=5

the calendar pagination would be perfect

Copy link

@ddnexus ddnexus Jan 7, 2024

Choose a reason for hiding this comment

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

i think that to make this redirect line work, we need to define @Calendar and all the collection here.

Yes, the @calendar should be created in the controller

if the url params were fixed ... the calendar pagination would be perfect

IIUC you would like to use params to get to any page by time, instead by number.

To me that looks actually more complicated. If you use pagy_calendar_url_at(@calendar, time) you get directly the path/url at that time instead of tinkering with 3 params separately.

Am I missing something?

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.

2 participants