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(events)] Add event type on the events.wordpress.org toppage #1352

Open
wants to merge 2 commits into
base: production
Choose a base branch
from

Conversation

hideokamoto
Copy link
Contributor

See #1197

Add the 'event type' on the upcoming event table at the top of events.wordpress.org.

Screenshots

スクリーンショット 2024-07-21 16 53 47

How to test the changes in this Pull Request:

  1. Clone this branch
    2.Follow the instruction to launch the test environment ( https://github.com/WordPress/wordcamp.org/blob/production/public_html/wp-content/themes/wporg-events-2023/README.md ) on this branch
  2. visit the top page ( http://localhost:8888 )

Text Domain: wordcamporg
Template: wporg-parent-2021
*/
* Theme Name: Events.WordPress.org 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like maybe a code editor has changed this - can it be reverted to the current version from the production branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh...
I'll revert this change.

Copy link
Contributor

@pkevan pkevan left a comment

Choose a reason for hiding this comment

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

Changes look good, but can we revert the change on style.css as per: https://github.com/WordPress/wordcamp.org/pull/1352/files#r1715378577

@hideokamoto
Copy link
Contributor Author

Thank you @pkevan for reviewing my PR!
I fixed the style.css file to revert the header comment.

Best.

@pkevan
Copy link
Contributor

pkevan commented Aug 22, 2024

Just tested this and it appears to hit a problem with a browser/screen width between 1279px - 961px.

See screenshot:

Screenshot 2024-08-22 at 12 22 59

@renintw
Copy link
Contributor

renintw commented Aug 26, 2024

@fcoveram 👋

In this PR, @hideokamoto tried adding the event type to the table. I recall we seemed to have discussed this before, but I can’t remember where. Do we need the event type in the table? I'm thinking adding the type might make things clearer and more consistent with the event widget in the admin dashboard. If we do need it, should the Desktop view follow the design in this PR?

event widget
image

As for the tablet and mobile views, do you have any thoughts?

Figma: https://www.figma.com/design/jdMk5ssz2Av7KFfEaeK7de/Events?node-id=1-2193&t=R04V8WEi9a8Hz58D-0

Current implementation from this PR (Still needs some further tweaks)

Desktop Tablet Mobile
image image image

@fcoveram
Copy link

I'd like to ping @dorsvenabili asking for her thoughts.

The suggestion makes sense to me, and I made this mockup for desktop and mobile views in case Rocio or team members agree with the change.

Mockups of events homepage on desktop and mobile

You can find here the designs, and the changes area pretty straightforward. The spacing between labels ([event-type] and [location]) and dot is 3x-small. This applies to both meetup and location, and date and time.

Mockup of event item in big and small size

I also noticed the row gap of li item is 20px instead of 40px (medium). Can we update that? The current spacing looks tight.

Row item with spacing notes from browser inspector

@dorsvenabili
Copy link
Collaborator

Thanks for the ping, @fcoveram! Yes, I confirm that showing the type of event makes a lot of sense and will help users to get a quicker sense of the type of event.

+1 to implement it :)

@renintw
Copy link
Contributor

renintw commented Sep 5, 2024

I think this PR currently has everything needed to continue with the implementation. @hideokamoto you can refer to the resources provided above. The position of the event type has changed, and attention is needed for different screen sizes.

Let me know if you run into any issues 🙂

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.

5 participants