Twitter oneboxes work well, except for quote tweets. In this case, the “parent” tweet is shown, but the quoted tweet is only shown as a link.
Would appreciate if someone could improve the oneboxing of quote tweets.
Twitter oneboxes work well, except for quote tweets. In this case, the “parent” tweet is shown, but the quoted tweet is only shown as a link.
Would appreciate if someone could improve the oneboxing of quote tweets.
I like this suggestion, its a rather complex change, but wacking a pr-welcome on it for now in case anyone is feeling brave.
I’ll try to make a PR to fix this. Will let you know
Still working on it but let me know if you have any suggestions/comments
PR is in the oven here:
https://github.com/discourse/onebox/pull/407
@techAPJ is reviewing.
There are some minor CSS things @awesomerobot may want to look at as well. In general we don’t like using :hover
and the rounded corners there look bigger than the ones on twitter, but that should be trivial to fix.
Overall this is looking great @jcalvento !
PR looks awesome @jcalvento!
Added a small comment regarding code formatting, once that is done it’s ready for merge.
This change is now live
Thanks @jcalvento
I made some changes to improve overall quote styling.
IMO, there is still a minor scope to reduce space between
The space is coming from white-space: pre-line
applied on tweet
class. @awesomerobot can you have a quick look if there is some scope for further improvement?
Still just wayyy too much space on mobile.
Right, we need that so the whitespace in the tweet is formatted as it was posted… but it also causes the whitespace in the HTML to show up, so here I removed it in the template — this should fix it
Deployed the updated gem and the onebox looks much better!
Should we consider decreasing padding on the top of timestamp to 5px
(from 10px
)?
Oh I think I missed some whitespace at the bottom… I think this will fix it
https://github.com/discourse/onebox/commit/cc4d02a3f7f8c54b154d8fa9cbdd741383da61e3
I dunno, I think Arpit is right when he says
Still too much whitespace here @awesomerobot
Hmm, there’s still an extra return in the template I missed somewhere… but removing the padding will get rid of the space despite that.
https://github.com/discourse/discourse/commit/d841bff9f8630ecb7ed711907022ff15f8d45d38