-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add "open in new tab" feature to Social Links Block #25468
Conversation
Size Change: +99 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
@mkaz I just saw this ticket was a duplicate and saw your reply in one of the older PR's #20740 (comment) I think with that we can close this aswell |
I was just reviewing this one and I'm not sure if the requiring only setting once is still worth holding out for, it is definitely the ideal scenario but something might be better than nothing. I'm not sure the efforts of allowing a parent block to pass down attributes to InnerBlock children. |
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.
I've tested this out and it works nicely.
@jasmussen we've had this before and overall, is it worth holding off the feature since it requires setting "open in new tab" on each link, or should we deploy. Ideal scenario is set once on parent and the children inherit.
@mkaz I took a stab at using the |
This implementation currently really only allows you to toggle |
Much better, I hadn't used the context yet. I had earmarked the ticket for when it got complete, but hadn't circled back. I've got a minor change, but I think this is what we want 👍 I'll let Joen take a look and make sure he's happy with the change. |
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.
Couple of minor changes.
Thanks again!
Co-authored-by: Marcus Kazmierczak <marcus@mkaz.com>
Co-authored-by: Marcus Kazmierczak <marcus@mkaz.com>
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.
👍
On a post that already contained a Social Icons block with 6 links, I'm getting 6 PHP notices:
@fabiankaegy @mkaz can you please have a look, when you have a chance? |
@afercia That is odd I had that in my checks. Will go over it again and have a look. My bad. Thank you for finding this |
Description
I have modified the
SocialLink
andSocialLinks
Blocks to include the ability to open in new tab. This was implemented using thecontextProvider
API to pass the setting down to the children.It fixes the issue outlined in #20707
Closes #20707
How has this been tested?
This has been tested locally with by setting up a SocialLinks block and then updating it to make sure there are no deprecation errors.
Types of changes
New feature & Bug fix
Checklist: