#684 | Add display options to "Event Date" block#1155
#684 | Add display options to "Event Date" block#1155mauteri merged 37 commits intoGatherPress:developfrom
Conversation
PR Summary
|
…ves dangling space
|
This is a cool thing and fun to play around with, thanks for your work @JordanPak ! I wouldn't have believed how much changes where needed, but I like your approach. A first look through your code was ok and seemed well done to me. The 3 unit tests are easily fixed, I think. I tried moving the Bildschirmaufzeichnung.vom.12.08.2025.11.01.21.webmOr even a little narrower with only two options Bildschirmaufzeichnung.vom.12.08.2025.16.14.47.webmOne thing I'm quite unsure with, is the wrong date formatting as you can see in the video. I don't know yet if this is a thing comming from your PR or if I messed something up elsewhere. |
src/blocks/event-date/edit.js
Outdated
| <SelectControl | ||
| label={__('Show time zone', 'gatherpress')} | ||
| value={showTimezone} | ||
| options={[ | ||
| { | ||
| label: sprintf( | ||
| /* translators: %s: Plugin "show timezone" setting */ | ||
| __( | ||
| '%s (plugin setting)', | ||
| 'gatherpress' | ||
| ), | ||
| globalShowTimezone | ||
| ? __('Yes', 'gatherpress') | ||
| : __('No', 'gatherpress') | ||
| ), | ||
| value: '', | ||
| }, | ||
| { | ||
| label: __('Yes', 'gatherpress'), | ||
| value: 'yes', | ||
| }, | ||
| { | ||
| label: __('No', 'gatherpress'), | ||
| value: 'no', | ||
| }, | ||
| ]} | ||
| onChange={(value) => | ||
| setAttributes({ showTimezone: value }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
I think this could be reduced to two options, while the default from settings will be
- shown in one of the labels (like currently) and
- define the pre-selected default if nothing is set on the block yet
Having this as only two options would allow to use ToggleControl for that whole thing.
A new ToggleControl could also be moved to the top directly behind the DateTimeRange component, which would bring timezone related settings closer to each other, even coming from different components technically.
Bildschirmaufzeichnung.vom.12.08.2025.11.38.01.webm
There was a problem hiding this comment.
Thanks for the review @carstingaxion!
I initially wanted it to be a toggle UI, but also wanted to make it possible to inherit the default or override to on or off. Would we be able to do that with a toggle?
There was a problem hiding this comment.
Why not? Or in other words, yes I think so.
Having a default as a separate option seems unnecessary to me. The two options yes and no will already include the default, so the default from settings will become the default for the block.
Like I showed in my demo.
There was a problem hiding this comment.
@carstingaxion a textbox/toggle is simpler to understand but would make it so the administrator can't choose to have all of the events inherit the setting and allow the existing events to respect it. For example, if the default option is empty/"" to just inherit the value (to display the timezone) from the plugin settings, then the administrator could enable or disable timezone display down the road without having to go back through every existing event post and enable/disable it.
It's a practical feature--the main issue is making it intuitive from a UI standpoint. @mauteri suggested adding some help text below to specify what "plugin settings" means and link to it. We could also think of better words to use instead of "plugin setting".
I guess this is related to #920 |
|
@JordanPak What do you think about the two variants of bringing the radio control for the display type into the Toolbar in the form of 2 or 3 buttons? |
I think the two-button setup in the block toolbar is pretty dope. I wonder if anyone won't realize that there's stuff in the block toolbar vs inspector controls, but that's a higher-level issue with the block editor itself. I wonder what everyone else's thoughts are, but I think the toolbar placement with buttons is better than the radios in the inspector controls. |
@carstingaxion I'm testing this implementation, but realized something that may or may not be concerning: The date format and separator controls in the inspector are conditional to the start/end/both setting, so if we move that to the toolbar, there's a bit more visual separation between the display setting and the separator + format settings when the display setting is changed, which might make it less intuitive? I'm going to try to present this to the group on Friday to gather a bit more feedback. |
… character removal
src/blocks/event-date/block.json
Outdated
| "default": "" | ||
| }, | ||
| "separator": { | ||
| "//": "@todo Add this to plugin settings", |
There was a problem hiding this comment.
@JordanPak I feel like we need to do this now because we have a lot of translated versions of GatherPress and feel the hardcoded to will be problem, or am I misunderstanding something here?
There was a problem hiding this comment.
@mauteri I think that todo was for having the default pull from the plugin settings so it'd be easy to change all of the existing blocks' separators at the same time while also respecting the language used in the setting.
This makes me realize, though, that unless we add extra depth to the setting (which probably wouldn't be ideal), how would would you be able to get one instance of a date block to have no separator unless the proposed setting were empty?
|
|
||
| let endFormat = dateFormat + ' ' + timeFormat + ' ' + timezoneFormat; | ||
| const parts = []; | ||
|
|
There was a problem hiding this comment.
@JordanPak what about something like this for translating the separator?
if ( 'to' === separator ) {
const translatedSeparator = __( 'to', 'gatherpress');
} else {
const translatedSeparator = separator;
}
There was a problem hiding this comment.
Oh rooiiight this'll prolly work @mauteri
Preview changes with PlaygroundYou can preview the recent changes for PR#1155 with the following PHP versions: PHP Version 8.3
PHP Version 7.4
Download Made with 💙 from GatherPress & a little bit of WordPress Playground. Changes will not persist between sessions. |
Description of the Change
Adds new display options to the
gatherpress/event-dateblock:Closes #684
How to test the Change
Go to an event post and add the "event date" block if it isn't already there. Select the block and change the new attribute inspector controls.
Changelog Entry
Credits
@JordanPak
Checklist: