Calendar: Add Border Support #64345
Open
Calendar: Add Border Support #64345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shail-mehta
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +32 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
t-hamano
reviewed
Aug 8, 2024
There was a problem hiding this comment.
Thanks for the PR!
My feeling is that, like with the Table block, border support should be reflected on the cells rather than the block itself.
Border support for the Table block works as follows:
e1789fcbbad75a414ef78ed6f107f1f7.mp4
This may require some complex modifications to achieve, but the source for the Table block should help.
There was a problem hiding this comment.
Thank you for working on this one @shail-mehta 👍
I'm wondering if the Calendar block might need a more nuanced approach to how it adopts border support.
If the block simply adopts border support and applies the styles to the block's wrapper, it might also need spacing support (at least padding).
The block currently uses the currentColor for the calendar's inner borders. As a user, I'd probably expect any border color selections I make to apply there.
Another question is whether the border should be around the header and navigation links or just the calendar's table. When the outer border is the same color as the inner borders and there is no padding between the calendar and the outer border, it looks like the border width is uneven due to the two borders coming together.
Given the varied questions, I think it would be great to get some design feedback here. I've taken the liberty of adding that label to this PR.
Comment on lines
+49
to
+54
| "__experimentalDefaultControls": { | ||
| "radius": true, | ||
| "color": true, | ||
| "width": true, | ||
| "style": true | ||
| } |
There was a problem hiding this comment.
Suggested change
| "__experimentalDefaultControls": { | |
| "radius": true, | |
| "color": true, | |
| "width": true, | |
| "style": true | |
| } |
The current thinking on whether border controls should show by default is that they should be optional for niche cases.
The best indicator for displaying border controls by default is whether the dimension controls are also displayed by default. With no padding or margin support on this block, I'd say we should hide these controls by default.
| @@ -1,5 +1,7 @@ | |||
| .wp-block-calendar { | |||
| text-align: center; | |||
| // This block has customizable border-box makes that more predictable. | |||
There was a problem hiding this comment.
Suggested change
| // This block has customizable border-box makes that more predictable. | |
| // This block has customizable borders. Border-box sizing makes that more predictable. |
There's a typo in this comment. Perhaps it should read something like above?
|
Thank you @t-hamano and @aaronrobertshaw for the Feedback. I will work on this PR and address the suggested changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Add border block support to the
Calendarblock.Part of #43247
Why?
Calendarblock is missing border support.How?
Adds the border block support in block.json.
Testing Instructions
Calendarblock's border is Configurable via Global Styles.Calendarblock and Apply the border Styles.Calendarblock styles take precedence over global Styles.Calendarblock borders display correctly in both the Editor and Frontend.Screenshots or Screencast
add-border-support-in-calendar.mp4