Remove newline to show FB and Twitter buttons on same line #4821 #4867
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…4821 Removing <br> tag was not simply enough, since facebook button root node is a div with display block. I am not that happy with this solution, because it relies on how Facebook renders its button, which can change at any time. Other solution I came up with was adding float:left and margin-right to twitter button, then it does not matter that Facebook button is block element, but still it relies on how twitter's script renders its button in iframe. Last solution I came up, but could not make work was wrapping both of them in divs and then position them based on our containers div with no regard what is inside.
|
Hi @publiclab/reviewers |
5 tasks
Generated by 🚫 Danger |
|
grvsachdeva
requested changes
Feb 21, 2019
Typo, closing tag. Co-Authored-By: swiatek7 <swiatek7@gmail.com>
|
I want to claim this. |
Done :) |
|
Merged 💯 👍 🎈 🎉 ! Hey @swiatek7, thanks for nice work. We would love to have more help from you. If you are interested in solving more issues then check here - https://code.publiclab.org/#r=all for a new issue. In case, you are unable to find an issue, feel free to comment here. Thanks! |
|
Hey @meet2410shah, please check other issues as this one is complete. Thanks! |
|
Thanks @gauravano :) |
icarito
pushed a commit
to icarito/plots2
that referenced
this pull request
Apr 9, 2019
…4821 (publiclab#4867) * Remove newline to show FB and Twitter buttons on same line publiclab#4821 Removing <br> tag was not simply enough, since facebook button root node is a div with display block. I am not that happy with this solution, because it relies on how Facebook renders its button, which can change at any time. Other solution I came up with was adding float:left and margin-right to twitter button, then it does not matter that Facebook button is block element, but still it relies on how twitter's script renders its button in iframe. Last solution I came up, but could not make work was wrapping both of them in divs and then position them based on our containers div with no regard what is inside. * Change solution to indepent of internals of social media buttons * Update app/views/sidebar/_dashboard.html.erb Typo, closing tag. Co-Authored-By: swiatek7 <swiatek7@gmail.com>
SrinandanPai
pushed a commit
to SrinandanPai/plots2
that referenced
this pull request
May 5, 2019
…4821 (publiclab#4867) * Remove newline to show FB and Twitter buttons on same line publiclab#4821 Removing <br> tag was not simply enough, since facebook button root node is a div with display block. I am not that happy with this solution, because it relies on how Facebook renders its button, which can change at any time. Other solution I came up with was adding float:left and margin-right to twitter button, then it does not matter that Facebook button is block element, but still it relies on how twitter's script renders its button in iframe. Last solution I came up, but could not make work was wrapping both of them in divs and then position them based on our containers div with no regard what is inside. * Change solution to indepent of internals of social media buttons * Update app/views/sidebar/_dashboard.html.erb Typo, closing tag. Co-Authored-By: swiatek7 <swiatek7@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4821
rake test@publiclab/reviewersfor help, in a comment belowRemoving <br> tag was not simply enough, since facebook button root node is a div with display block.I am not that happy with this solution, because it relies on how Facebook renders its button, which can change at any time.
Other solution I came up with was adding float:left and margin-right to twitter button, then it does not matter that Facebook button is block element, but still it relies on how twitter's script renders its button in iframe.
Last solution I came up, but could not make work was wrapping both of them in divs and then position them based on our containers div with no regard what is inside.
I have managed to implement this feature with wrapping both buttons in divs. Not it is independent from the way twitter or facebook renders its buttons.