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

Set fixed height of tile in photos stream, fix #6809 #6838

Merged
merged 1 commit into from Aug 8, 2016

Conversation

Projects
None yet
3 participants
@Flaburgan
Copy link
Member

commented May 18, 2016

I wanted to go with a beautiful masonry to fix this problem, but I think it's more important to release 0.6 first before making other design changes so I just set up a fixed height for the tile and vertically align the images.

text-align: center;
white-space: nowrap;

&::before {

This comment has been minimized.

Copy link
@diaspora-code-review

diaspora-code-review May 18, 2016

Selector should have depth of applicability no greater than 3, but was 4

vertical-align: middle;
}

.big-photo {

This comment has been minimized.

Copy link
@diaspora-code-review

diaspora-code-review May 18, 2016

Selector should have depth of applicability no greater than 3, but was 5

display: inline-block;
height: 100%;
vertical-align: middle;
}

This comment has been minimized.

Copy link
@Flaburgan

Flaburgan May 18, 2016

Author Member

This is a famous technic (see http://stackoverflow.com/a/7310398) but it produces a small offset to the right here. Not sure why, maybe @svbergerem you have an idea?

This comment has been minimized.

Copy link
@svbergerem

svbergerem Aug 8, 2016

Member

margin-right: -4px; fixes this but I don't know where the offset comes from.

@svbergerem

This comment has been minimized.

Copy link
Member

commented Aug 8, 2016

@Flaburgan I think I'd prefer a full-height (200px) and full-width (100%) link with a centered image inside. Currently the link is just as big as the image itself. What do you think about that?

@Flaburgan

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2016

I'm going to make the whole thumbnail clickable. Pronto is not happy but I want this patch to be a quick fix, not a big refactor so I propose to let the code like it is.

@Flaburgan Flaburgan force-pushed the Flaburgan:masonry-photo branch 4 times, most recently from eaaefed to e1cbbd2 Aug 8, 2016

&:hover,
&:focus,
&:active {
border-color: $light-grey;

This comment has been minimized.

Copy link
@svbergerem

svbergerem Aug 8, 2016

Member
+ text-decoration: none;

Otherwise you will see a small blue line next to some small images.


.big-photo {
display: inline;
max-height: 200px;

This comment has been minimized.

Copy link
@svbergerem

svbergerem Aug 8, 2016

Member
app/assets/stylesheets/stream_element.scss:56 W: Properties should be ordered display, margin-left, max-height, vertical-align

@Flaburgan Flaburgan force-pushed the Flaburgan:masonry-photo branch from e1cbbd2 to eac051f Aug 8, 2016

}

.big-photo {
display: inline;

This comment has been minimized.

Copy link
@diaspora-code-review

diaspora-code-review Aug 8, 2016

Selector should have depth of applicability no greater than 3, but was 4

margin-left: -4px;
max-height: 200px;
vertical-align: middle;
}
}
}

This comment has been minimized.

Copy link
@diaspora-code-review

diaspora-code-review Aug 8, 2016

Selector should have depth of applicability no greater than 3, but was 4

@svbergerem svbergerem merged commit eac051f into diaspora:develop Aug 8, 2016

5 of 6 checks passed

pronto/scss Found 3 warnings.
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 82.56%
Details
pronto/es_lint Coast is clear!
pronto/haml Coast is clear!
pronto/rubocop Coast is clear!

svbergerem pushed a commit that referenced this pull request Aug 8, 2016

Steffen van Bergerem
Merge pull request #6838 from Flaburgan/masonry-photo
Set fixed height of tile in photos stream, fix #6809
@svbergerem

This comment has been minimized.

Copy link
Member

commented Aug 8, 2016

Thank you.

@Flaburgan Flaburgan deleted the Flaburgan:masonry-photo branch Aug 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.