Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
An alter is required to change or add new attributes to the social share span or link. For example title in the span tag.
Comment | File | Size | Author |
---|---|---|---|
#28 | 2900290-28.patch | 3.89 KB | akalata |
#27 | 2900290-sharethis-beta5-an_alter_required_for-27.patch | 3.79 KB | ankitv18 |
| |||
#18 | an_alter_required_for-2900290-18.patch | 3.77 KB | bibliophileaxe |
#7 | an_alter_required_for-2900290-7.patch | 3.51 KB | gg24 |
|
Comments
Comment #2
gbisht CreditAttribution: gbisht as a volunteer and at Acquia commentedCreated a patch with the alter to update or add new elements to the attribute variable.
Comment #3
gbisht CreditAttribution: gbisht as a volunteer and at Acquia commentedPatch for latest stable version 8.x-2.0-beta3
Comment #4
gbisht CreditAttribution: gbisht as a volunteer and at Acquia commentedImproved patch for 8.x-2.0-beta3 version.
Comment #5
joshi.rohit100Inject module handler.
Comment #6
gbisht CreditAttribution: gbisht as a volunteer and at Acquia commentedPatch updated and module handler injected.
Comment #7
gg24 CreditAttribution: gg24 as a volunteer and at QED42 commentedHi,
I am adding alter hook for attribute variable and data options both. As we are extending it for attributes, it is good to extend it for data options as well. Along with this I am adding sharethis.api.php as well which is missed in the above patch.
Please review.
Thanks!
Comment #8
purushotam.rai CreditAttribution: purushotam.rai at QED42 commentedIn my opinion, it will be a good to have this option specifically, when more generalized. Marking it RTBC
Thanks and Regards
Comment #9
naveenvalechaAs the service provided by the module is public and there shouldn't be any BC break in its constructor, so any custom module decorated the existing service will be broken to add a new getter method and inject the module handler service by calling this explicitly in the constructor.
I'm fine with adding this F/R. but adding this as this seems to maintain an edge case.
Comment #10
purushotam.rai CreditAttribution: purushotam.rai at QED42 commented@naveenvalecha could you please elaborate on what point u r stressing over here.
Comment #11
purushotam.rai CreditAttribution: purushotam.rai at QED42 commented@naveenvalecha I guess, if any custom module uses this service, then there comes 2 scenario:
1. Service in Module Hook
2. Service in a Plugin/Another Service (PHP Class)
In both cases, we don't need to explicitly create arguments for the constructor of this service. Either we will use Drupal service or inject this into the class which definitely won't be broken.
Kindly let me know if we are on the same page or I'm missing something.
Thanks and Regards
Comment #12
purushotam.rai CreditAttribution: purushotam.rai at QED42 commentedComment #13
purushotam.rai CreditAttribution: purushotam.rai at QED42 commentedComment #14
purushotam.rai CreditAttribution: purushotam.rai at QED42 commentedI think container factory will take care of initialization.
Comment #15
purushotam.rai CreditAttribution: purushotam.rai at QED42 commentedComment #16
andypostI think it totally fine to change arguments of service, and module should allow to alter default render somehow
Comment #17
timodwhit CreditAttribution: timodwhit commentedThe patch works fine, but there seems to be an issue with the module when selecting which buttons to alter. Example: I needed to add specific text for a couple services, but as I could tell the service was not passed to this alter hook. Here is an example of what I had to to:
There are couple things that are problematic here:
Otherwise the patch is great, I think that the general theming aspect of this module needs a little work.
Comment #18
bibliophileaxeWe had a requirement to render text in the span tag - specifically the title of the service. I have added the span text to the alter hook as well.
Comment #19
andypostI still think that better to pass whole render element here to allow altering the build array
Comment #20
naresh_bavaskar#18 applied successfully and works fine for me.
unassigning @naveenvalecha..
Comment #21
ankitv18 CreditAttribution: ankitv18 as a volunteer and commentedRe-roll the patch for latest version 2.0.0-beta5
Removing the unused statements from SharethisManager
Review patch #21
Comment #23
ankitv18 CreditAttribution: ankitv18 as a volunteer and commentedFixed the phpcs issues.
Patch #23
Comment #25
ankitv18 CreditAttribution: ankitv18 as a volunteer and commentedIgnore the patch #23
Missed space, not caught up in phpcs
Patch #25
Comment #27
ankitv18 CreditAttribution: ankitv18 as a volunteer and commentedRemoving the renderer plugin from service file and re-roll the patch to work with 2.0.0-beta5
Review Patch# 27
Comment #28
akalata CreditAttribution: akalata at Tag1 Consulting commentedUpdates
hook_sharethis_render_alter()
documentation as the$share_text
is also alterable.Comment #29
VladimirAusRequires rerolling...
Comment #30
mrinalini9 CreditAttribution: mrinalini9 at Srijan | A Material+ Company for Drupal India Association commentedPatch #28 applied cleanly on the 3.0.x branch, so I think reroll is not needed.
Thanks & Regards,
Mrinalini
Comment #31
VladimirAusThank you! Committed! 🍸
Comment #32
VladimirAus