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>

CommentFileSizeAuthor
#14 add-username-setting-2128033-14.patch10.37 KBdevin carlson
#8 twitter_block-add_twitter_account_setting-2128033-8.patch6.63 KBmfernea
#5 twitter_block-add_twitter_account_setting-2128033-5.patch6.63 KBgboudrias
#4 twitter_block-add_twitter_account_setting-2128033-4.patch6.46 KBmfernea
#1 twitter_block-add_twitter_account_setting-2128033-1.patch5.82 KBmfernea

Comments

mfernea’s picture

mfernea commented
StatusFileSize
new twitter_block-add_twitter_account_setting-2128033-1.patch5.82 KB

I'm posting a patch for this.

mfernea’s picture

mfernea commented
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

mfernea commented
Status: Needs work » Needs review
StatusFileSize
new twitter_block-add_twitter_account_setting-2128033-4.patch6.46 KB

I've updated the tests also.

gboudrias’s picture

gboudrias commented
StatusFileSize
new twitter_block-add_twitter_account_setting-2128033-5.patch6.63 KB

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

mfernea commented

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

gboudrias commented

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

mfernea commented
StatusFileSize
new twitter_block-add_twitter_account_setting-2128033-8.patch6.63 KB

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

gboudrias’s picture

gboudrias commented
Status: Needs review » Reviewed & tested by the community

Thanks :)

ladybug_3777’s picture

ladybug_3777 commented

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

mfernea commented
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

gboudrias commented

I vote for this one too :)

Can anyone commit this?

ladybug_3777’s picture

ladybug_3777 commented

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

devin carlson’s picture

devin carlson commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new add-username-setting-2128033-14.patch10.37 KB

An updated patch with a few minor fixes.

devin carlson’s picture

devin carlson commented
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

ladybug_3777 commented

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

ladybug_3777 commented

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

mfernea commented

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

ladybug_3777’s picture

ladybug_3777 commented

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.