I think it would be usefull to have a setting: twitter_account. This would allow editors to add the Twitter account for which the widget is added.
This will be used to replace:
<a href="https://twitter.com/twitterapi">Tweets by @twitterapi</a>
with
<a href="https://twitter.com/twitter_account">Tweets by @twitter_account</a>

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mfernea’s picture

I'm posting a patch for this.

mfernea’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: twitter_block-add_twitter_account_setting-2128033-1.patch, failed testing.

mfernea’s picture

Status: Needs work » Needs review
FileSize
6.46 KB

I've updated the tests also.

gboudrias’s picture

You're missing the element that actually changes the account in the feed. I don't know if that's intentional? I rerolled the patch while adding this in the link options:

'data-screen-name' => $config['twitter_account'],

Works for me.

mfernea’s picture

Thanks for help! I saw that you also improved the description of the field. But you set the field as not required. Is this intentional? As I see it, it has to be required.

gboudrias’s picture

Argh, I knew I'd forgotten something... No it's not intentional... I don't know if it's worth submitting another patch, I'm sure the module maintainers can correct my mistake easily enough.

mfernea’s picture

I'm uploading a new one, just to be sure it's fine. :)

gboudrias’s picture

Status: Needs review » Reviewed & tested by the community

Thanks :)

ladybug_3777’s picture

I just wrote a patch that will allow users to override the twitter username that is being displayed. Perhaps we should combine my patch with this one? (Sorry I didn't see this issue before writing my patch)

https://drupal.org/node/2230823

mfernea’s picture

Assigned: mfernea » Unassigned

The main difference between the two patches is that this one uses an extra column in the db. I would go for this one since it's already RTBC.
If it's considered useful we can merge the two patches.

gboudrias’s picture

I vote for this one too :)

Can anyone commit this?

ladybug_3777’s picture

FYI, I tested this patch and it works wonderfully, I also vote for it to be committed!

Devin Carlson’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
10.37 KB

An updated patch with a few minor fixes.

Devin Carlson’s picture

Status: Needs review » Fixed

Tested #14 and committed to Twitter Block 7.x-2.x.

I'll roll a new release shortly.

  • Devin Carlson committed 18468ef on 7.x-2.x authored by mfernea
    Issue #2128033 by mfernea, gboudrias, ladybug_3777, Devin Carlson: Added...
ladybug_3777’s picture

This new patch #14 does not work the same as patch #8! I'm disappointed that patch #14 is what was added to version 2.2 instead of version #8 because patch #14 doesn't allow you to override the twitter username using the data-screen-name attribute.

'data-screen-name' => $config['twitter_account'],

Why was that part removed?

I wish the community had been given a chance to provide input on the newer patch before it was committed to a major release. I feel like it didn't get a chance to be properly tested either IMHO.

ladybug_3777’s picture

New issue and patch added here https://www.drupal.org/node/2300407 for those that are looking to get this functionality back.

mfernea’s picture

Indeed, there's a slip-up. I'll have a look at the patch in the new ticket.

ladybug_3777’s picture

I agree, it is a slip-up. Just a heads up, my new issue has been modified without my prior knowledge. My patch is still there and it still gives back the functionality that was lost, but the title and my description has been changed by someone else. It can be confusing when you link over (although mfernea I think you linked over and read it before it was modified, thanks!)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.