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

Feat: redesign events page ✨ #13374

Merged
merged 20 commits into from
Aug 12, 2024
Merged

Conversation

saurabhburade
Copy link
Contributor

@saurabhburade saurabhburade commented Jul 10, 2024

Description

  • Redesign EventCard
  • Redesign UpcomingEventsList
  • Redesign MeetupList

Preview URL

https://deploy-preview-13374--ethereumorg.netlify.app/en/community/events

Related Issue

Fixes #13367

@github-actions github-actions bot added the tooling 🔧 Changes related to tooling of the project label Jul 10, 2024
Copy link

netlify bot commented Jul 10, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit f85cb38
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/66b0a8080502b800083b913b
😎 Deploy Preview https://deploy-preview-13374--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 45 (🔴 down 6 from production)
Accessibility: 93 (no change from production)
Best Practices: 89 (🔴 down 10 from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@saurabhburade
Copy link
Contributor Author

Hi @konopkja, please check the preview. Also please let me know if any modifications required

@konopkja
Copy link
Contributor

Wow! looking good, there might be few minor adjustments with the interaction but overall this is shaping up pretty well! Thank you.

  1. could you please double the padding for Month headline? (right now month headline is too close to the cards from previous month)

  2. Can we load by default 2 months instead of one?

  3. The button should also load 2 months more instead of one to prevent excessive need to click on the button

@saurabhburade
Copy link
Contributor Author

saurabhburade commented Jul 11, 2024

Thank you! surely, making the required modifications.

@konopkja
Copy link
Contributor

Thank you! surely, making the required modifications.

Lgtm, thank you!!

Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Hey @saurabhburade! Thanks for the work here, appreciate this. It's overall looking great! I left a note about concerns for non-English pages with the proposed logic for parsing the display date range.

Let me know if what I mentioned makes sense, but ideally these can be displayed properly according to the users locale, and not be forced to just English.

src/components/UpcomingEventsList.tsx Outdated Show resolved Hide resolved
src/components/EventCard.tsx Outdated Show resolved Hide resolved
src/components/UpcomingEventsList.tsx Outdated Show resolved Hide resolved
src/lib/utils/date.ts Outdated Show resolved Hide resolved
@konopkja
Copy link
Contributor

a small UI change requests after team review:

  1. remove year from each event date (month and day should be enough)
  2. increase the padding around the button "load more" button as it seems a bit hidden now

@saurabhburade
Copy link
Contributor Author

@konopkja @wackerow Thank you for your feedback, here's the modified implementation for dates
Screenshot 2024-07-24 at 5 24 57 PM

@konopkja
Copy link
Contributor

@konopkja @wackerow Thank you for your feedback, here's the modified implementation for dates Screenshot 2024-07-24 at 5 24 57 PM

why is there the "ago" part?

@wackerow
Copy link
Member

@konopkja I'm assuming this is showing Italian ("August" is "agosto", shortened to "ago")

@saurabhburade
Copy link
Contributor Author

saurabhburade commented Jul 24, 2024

It's a locale date-time for es-ES

Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Looking good @saurabhburade! Mind clearing the merge conflicts an we'll give this a final review?

src/components/EventCard.tsx Outdated Show resolved Hide resolved
@github-actions github-actions bot added content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels Jul 29, 2024
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Hey @saurabhburade, sorry to block again. Noted a couple things:

  1. Clamping lines leads to unreachable content
  2. Can use Intl.DateTimeFormat(locale,options).formatRange(from,to) to get date range string in a simple, easy to maintain, an intl friendly manner
  3. to is now deprecated; use href

I kinda see 2 and 3 as blocking, curious your take @konopkja

src/components/EventCard.tsx Show resolved Hide resolved
src/lib/utils/date.ts Outdated Show resolved Hide resolved
@saurabhburade
Copy link
Contributor Author

@wackerow @konopkja

Clamping lines are to evenly show the event cards, if not it may lead to uneven heights. As the length of the description of an event is unpredictable

@wackerow
Copy link
Member

@saurabhburade Gotcha! In that case I'd consider a css grid which would allow you to have cards in a row all have the same height, but have that height be flexible depending on the longest card text for that row

@wackerow wackerow added needs design approval 🧑‍🎨 Approval from a designer is needed before merging and removed needs review 👀 labels Aug 2, 2024
Copy link
Contributor

@nloureiro nloureiro left a comment

Choose a reason for hiding this comment

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

All good from the design side!!!
Great work!

@nloureiro
Copy link
Contributor

@saurabhburade, great work, thank you.

one last thing, I got this error on Safari.
Screen Shot 2024-08-05 10 56 47 AM

@saurabhburade
Copy link
Contributor Author

@nloureiro, thank you. The safari issue is fixed now.

@saurabhburade
Copy link
Contributor Author

@wackerow Please do the final review, LGTM for approval.

@wackerow
Copy link
Member

Looks great, thank @saurabhburade! Can continue to iterate on some other upgrades here, and the tailwind migration separately

@wackerow wackerow merged commit e7ca8c6 into ethereum:dev Aug 12, 2024
7 of 10 checks passed
This was referenced Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits needs design approval 🧑‍🎨 Approval from a designer is needed before merging tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Events page redesign
4 participants