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

List of Events #118

Merged
merged 22 commits into from
Oct 2, 2023
Merged

List of Events #118

merged 22 commits into from
Oct 2, 2023

Conversation

L-Steinmacher
Copy link
Contributor

@L-Steinmacher L-Steinmacher commented Sep 26, 2023

Adding a list of past Seattle JS events that routes to the individual event page.

@L-Steinmacher L-Steinmacher marked this pull request as ready for review September 27, 2023 23:52
@L-Steinmacher
Copy link
Contributor Author

@andrewiggins and @crtr0 This pull request is ready for review.

@crtr0 crtr0 changed the title Lls List of Events Sep 28, 2023
@crtr0
Copy link
Member

crtr0 commented Sep 28, 2023

Thanks Lucas. Can you please attach screenshots of the pages that you've added or altered to this PR?

@L-Steinmacher
Copy link
Contributor Author

home /

I added a link to the past events (/events) list under the current events info.

Screen Shot 2023-09-28 at 9 42 41 AM

/events

There is no major styling on this page and I wanted to chat with what you envisioned. I noticed most links still had default styling on the page so I left then as is.

Screen Shot 2023-09-28 at 9 42 27 AM

/events/$id

I copied pasta styles to display speaker images and talk info as well as I left sponsors on the page. I also displayed the event description but may remove it since it, for the most part, will just mention the price of the early bird. The other thought was just change the description in the data.json to whatever we want.

Screen Shot 2023-09-28 at 9 42 11 AM

@crtr0
Copy link
Member

crtr0 commented Sep 28, 2023

Looks good! Can you please figure out how to left-align the list of events so that it lines up with the heading?

}
@media only screen and (min-width: 768px) {
:host {
display: flex;
Copy link
Contributor

Choose a reason for hiding this comment

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

The list of events looks a little funky on a desktop browser. It's not aligned with the title. Probably not a blocking issue, but is there an easy fix?
image

Copy link
Contributor

Choose a reason for hiding this comment

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

whoops, Carter already mentioned this here: #118 (comment)

@L-Steinmacher
Copy link
Contributor Author

L-Steinmacher commented Sep 28, 2023

Looks good! Can you please figure out how to left-align the list of events so that it lines up with the heading?
@crtr0 @FX-Wood
Fixed alignment.
Screen Shot 2023-09-28 at 12 13 10 PM

@crtr0
Copy link
Member

crtr0 commented Sep 28, 2023

Last nitpick - can you please not display a future event (i.e. the Oct 2023 event) on the "Past Events" page?

@L-Steinmacher
Copy link
Contributor Author

Last nitpick - can you please not display a future event (i.e. the Oct 2023 event) on the "Past Events" page?
@crtr0
😅 Yeah that makes total sense. I corrected that just now in the latest commit but noticed something strange while fixing it. The dates for 2022 are not in order! I looked at the data/events.json and saw that the dates for the May-October 2022 events are the same. I didn't want to go change the data without checking in first. I can go ahead and manually fix the data if you'd like.

@crtr0
Copy link
Member

crtr0 commented Sep 28, 2023

Yes, please fix the dates. Second Wednesday of the month for all of them should be correct (or close enough).

@L-Steinmacher
Copy link
Contributor Author

Event dates are fixed in the json. @crtr0

Copy link
Member

@andrewiggins andrewiggins left a comment

Choose a reason for hiding this comment

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

Hey @L-Steinmacher, thanks for doing this! We've long needed this page :) Left you some thoughts

app/elements/events-list.mjs Outdated Show resolved Hide resolved
app/elements/events-list.mjs Outdated Show resolved Hide resolved
app/elements/events-list.mjs Show resolved Hide resolved
app/elements/event-sponsors.mjs Outdated Show resolved Hide resolved
app/pages/talks.mjs Outdated Show resolved Hide resolved
app/pages/talks.mjs Outdated Show resolved Hide resolved
app/elements/event-talk.mjs Outdated Show resolved Hide resolved
app/pages/events/$id.mjs Outdated Show resolved Hide resolved
Copy link
Member

@andrewiggins andrewiggins left a comment

Choose a reason for hiding this comment

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

Thanks for the progress here! Just a few more comments and we should be good to go!

app/api/events/$id.mjs Outdated Show resolved Hide resolved
app/elements/event-talk.mjs Outdated Show resolved Hide resolved
app/pages/events/$id.mjs Outdated Show resolved Hide resolved
app/pages/events/$id.mjs Outdated Show resolved Hide resolved
app/api/events/$id.mjs Outdated Show resolved Hide resolved
Copy link
Member

@andrewiggins andrewiggins left a comment

Choose a reason for hiding this comment

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

Thanks for this update and dealing with all of my suggestions <3 We've needed this a while. I really appreciate you doing this for us

@andrewiggins andrewiggins merged commit 8051d8f into seattlejs:main Oct 2, 2023
2 checks passed
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.

4 participants