search

columbus-elixir / marketing

12 stars 8 forks source link

Meeting page #62

Closed Codball closed 5 years ago

Codball commented 5 years ago

Referencing issue #58 Added functionality for past meetings. Shows paginated meetings containing the date, speakers and their talk title. Mostly compatible with mobile. The only thing remaining is how to implement showing a link to the meetings page on the homepage.

Requires an interesting workaround for paginating a struct with preloaded associations. See comment in meetings_controller.ex.

2019-02-0353

chrome_2019-01-28_16-49-24

croz1007 commented 5 years ago

@Kartstig @Codball I can't seem to locate this branch when I do a git fetch so I cannot even pull it locally to try and get it running. Hmmm, I'll check again later. Also, can you please provide screenshots and/or gifs of the screens and a link to the issue this is related to as part of the description for this PR?

Kartstig commented 5 years ago

@croz1007 you'll have to add the remote for @codball 's fork. We don't give push access to public users

croz1007 commented 5 years ago

Still trying to get it running. Looks like I may have a lot of npm updates to make. We'll see.

croz1007 commented 5 years ago

Got it running locally and reviewing now.

Codball commented 5 years ago

At the moment pagination breaks when you add multiple speakers. The query meetings_for_page/2 in App.Meetings.ex returns the correct information, but when passed to paged_query/3 in Pagination.ex the results returned are mixed up in an odd way, I can't recognize a pattern. Is it counting a speaker as part of the limit? I'm thinking I could grab the speakers in a different way, perhaps in another query or preloading them after the query is paginated.

Kartstig commented 5 years ago

Some notes:

For the Next and Previous buttons, you can try to use FontAwesome. Here's a great one for Next: https://fontawesome.com/icons/angle-right?style=solid

Then you just have to do the following:

<i class="fas fa-angle-right"></i>
Kartstig commented 5 years ago

When you add tests, make sure you make changes to prevent seeds from running multiple times in your database: https://github.com/columbus-elixir/marketing/pull/64

Then you can do length based validations. Otherwise, you'll have tons of meetings shoved into your test db after running it many times.

Kartstig commented 5 years ago

@Codball Your tests should pass locally now if you pull from master

Kartstig commented 5 years ago

Do we want to put a link to this page from index?

croz1007 commented 5 years ago

@Kartstig @Codball I think that a link on the index page would be great. I don't believe that it is a top nav type of link, but rather a link down below the "This Month's Speakers" section with a heading of "Past meet ups" or something like that. Looks like you have a heading there in your screenshots.

Just a thought about that link: Could either be just a link to that /meetings page, or we could list out the dates of the last 3 as links to those meetings and a generic link to see all past meetings. I guess that we are not doing a show page for a specific meeting at this point so we would not be able to link to them individually right now.

Also, Nathan, I think the updated screenshots look really great.

ieatkimchi commented 5 years ago

looks good, fyi there is an ecto lib for pagination. https://github.com/drewolson/scrivener_ecto