Needs review
Project:
Twitter
Version:
7.x-5.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
15 Feb 2012 at 22:11 UTC
Updated:
29 Oct 2015 at 10:55 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
danielphenry commentedMy implementation of tokens module support for twitter post module.
Comment #2
davidneedhamThis is definitely still an issue and something I'd like to see implemented as well. Can you re-roll against the 7.x-5.x-dev?
Comment #3
13rac1 commentedSet #370980: Include node type in post announcement as duplicate.
Comment #4
foxtrotcharlie commentedHere's a reroll. The original patch actually succeeded on 7.x-5.x-dev (with fuzz - just some line number changes). Here is the patch rerolled against 7.x-5.x-dev. Not quite sure if the interdiff is correct, but posting it anyway... I haven't been able to actually test that the functionality works, but at least we have an updated patch.
Comment #5
foxtrotcharlie commentedI think that this patch doesn't work as intended - I manually applied it to my 7.x-5.8 version of the twitter module, but it then gave me the token help at the top of my node/add forms (see attached screenshot). I think it may be that the form key is incorrect, so instead of $form['token_help'] it should be $form['twitter']['token_help'] to appear in the correct place? It also seemed to load really slowly and kept telling me that a script had become unresponsive, but not sure if that is related to this change.
I will do some testing when I get a chance, but I think needs work.
Comment #6
damienmckennaYou should also use the '#dialog' => TRUE option in the form field to make it load in a popup instead of inline.
Comment #7
foxtrotcharlie commentedThanks for the tip @Damien. I have updated the patch. It now displays the token help in a fieldset, and the tokens open up in a dialog. To hide and show the token replacements fieldset I had to update the twitter_post javascript. I don't actually know if the fieldset "REPLACEMENT PATTERNS" is necessary - I got it from the pathauto module. I also haven't tested whether or not the tokens are replaced correctly. Will get to that when I get some more time. Leaving it as 'needs work' because it probably does...
Comment #8
foxtrotcharlie commentedI have done some testing - created content and added tokens to the post to twitter field, and the tokens were rendered correctly in the tweet. I also set some default tokens in the twitter post admin page, and these were also tweeted correctly. UI feels ok to me.
Comment #9
intrafusionI'm not sure how many people need to test it before RTBC can be set as the status, but this appears to work well.
I have 2 comments to make: Firstly as the Token module has over 800,000 installs (3rd behind Views & Ctools in terms of number of installs) should token be a requirement and drop the module's own replacements? Secondly, should the tinyurl replacement be dropped as all links submitted within tweets and direct messages, regardless of length, will be wrapped with t.co
Comment #10
damienmckenna@intrafusion: I don't think making it optional is that much of a deal, and it's definitely worth removing the URL shortening.
Comment #11
intrafusionI have taken @foxtrotcharlie's patch from #7 and updated it by removing !tinyurl and altering the fields & descriptions to use token replacements rather than the module's in-built replacements.
Comment #13
damienmckennaLets drop the token support and move the remove-TInyURL stuff into #2555819: Remove TinyURL integration.
Comment #15
damienmckennaComment #17
intrafusion@DamienMcKenna what's the status of this issue? What needs to be done to get this into the module?
Comment #18
damienmckennaThe update script needs to do a search/replace on the variable, rather than force-update it to a new string.
I'm contemplating dropping the 7.x-5.x branch and focusing effort on the update script to 7.x-6.x, because it already has token support.