-
Notifications
You must be signed in to change notification settings - Fork 4
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
Improve UI #74
Improve UI #74
Conversation
d763e92
to
71cfec0
Compare
@trymebytes If you want I can resolve conflicts here, they're due to the linting PRs If you're not currently actively working on this branch, I would be happy to rebase with trunk and force-push here with conflicts fixed. |
Sure, please go ahead. Thank you! |
Rebased with trunk to fix conflicts. In case I messed something up, there's a copy of the original branch at https://github.com/WordPress/wporg-gp-translation-events/tree/improve-ui-backup |
3a785ba
to
439caba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better!
<div> | ||
<details class="event-stats-summary"> | ||
<summary>View stats summary in text </summary> | ||
<p class="event-stats-text"><?php echo esc_html( sprintf( 'At the %s event, %d people contributed in %d languages (%s), translated %d strings and reviewed %d strings.', esc_html( $event_title ), esc_html( $event_stats->totals()->users ), count( $event_stats->rows() ), esc_html( implode( ',', array_keys( $event_stats->rows() ) ) ), esc_html( $event_stats->totals()->created ), esc_html( $event_stats->totals()->reviewed ) ) ); ?></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider breaking this line up as it's really long, which makes it hard to read, IMHO.
<p> | ||
Host: <a href="<?php echo esc_attr( get_author_posts_url( $event->post_author ) ); ?>"><?php echo esc_html( get_the_author_meta( 'display_name', $event->post_author ) ); ?></a> | ||
<?php if ( current_user_can( 'edit_post', $event_id ) ) : ?> | ||
<a class="event-page-edit-link button" href="<?php echo esc_url( gp_url( 'events/edit/' . $event_id ) ); ?>"><span class="dashicons dashicons-edit"></span>Edit event</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, weird 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icon is still not showing but I couldn't figure out why. I tested on firefox and chrome and it doesn't show in either.
Could you also improve the date formatting? Maybe split into start and end or show short dates and times but right now it's not very clear and feels convoluted. |
e74c48e
to
a60434a
Compare
<?php endif; ?> | ||
</div> | ||
<?php if ( ! empty( $event_stats->rows() ) ) : ?> | ||
<?php if ( ! empty( $event_stats->rows() ) ) : ?> | ||
<div class="event-details-stats"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #65 and the overall look and feel of other pages.
Screenshots
Before
After