Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite hover effect on conversations with multiple participants in css #5776

Merged
merged 2 commits into from
Mar 14, 2015

Conversation

lislis
Copy link
Contributor

@lislis lislis commented Mar 14, 2015

We noticed that the sliding effect on conversations with a lot of participants has kind of poor performance. I rewrote the effect in css which looks way smoother and isn't that annoying once you have a lot on these conversations.
I had to cherry pick the commit from another fork and adjust it to a backbone refactor. Sry for the weird git things, it made me go crazy.
Opinions and suggestions welcome :D

Conflicts:
	app/assets/javascripts/inbox.js
	app/assets/stylesheets/conversations.css.scss
@jhass
Copy link
Member

jhass commented Mar 14, 2015

Looks nice! If you could just indeed clean up the git history a bit? git checkout rewrite-conversation-hover-in-css; git rebase -i e2a5912;, there choosing squash for your last two commits and then just git push -f origin rewrite-conversation-hover-in-css.

@lislis lislis force-pushed the rewrite-conversation-hover-in-css branch from 11fb385 to ee5bcd2 Compare March 14, 2015 15:14
@svbergerem
Copy link
Member

Great! :) Unfortunately conversations with more than 2 participants now have a bigger bottom-margin. It would be great if you could fix that.

The participants div has some additional padding-top and margin-top. Only adding that for hovered conversations and setting it to 0 for all others should fix that. Maybe you could also add a transition for those two properties.

update Changelog

remove weird merge thing

think of the padding and margin
@lislis lislis force-pushed the rewrite-conversation-hover-in-css branch from ee5bcd2 to a11322f Compare March 14, 2015 16:00
@lislis
Copy link
Contributor Author

lislis commented Mar 14, 2015

@svbergerem ha you're right :) I just assumed everything uses border-box. I pushed the fix

@svbergerem svbergerem added this to the next-major milestone Mar 14, 2015
@svbergerem svbergerem merged commit a11322f into diaspora:develop Mar 14, 2015
svbergerem pushed a commit that referenced this pull request Mar 14, 2015
Rewrite hover effect on conversations with multiple participants in css

Conflicts:
	Changelog.md
@svbergerem
Copy link
Member

Awesome, the hover effect feels much more mature with this PR. Thank you! :)

@lislis lislis deleted the rewrite-conversation-hover-in-css branch March 14, 2015 16:41
@lislis
Copy link
Contributor Author

lislis commented Mar 14, 2015

Yaay!

@goobertron
Copy link

Thanks a lot, @lislis!

@Flaburgan
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants