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

Fix image stretching #544

Merged
merged 3 commits into from
Aug 22, 2015
Merged

Fix image stretching #544

merged 3 commits into from
Aug 22, 2015

Conversation

geekgonecrazy
Copy link
Contributor

By setting height to 200 you were causing images to stretch. Setting it to a max-height, makes it fit auto resize to fit your max height.

This would fix #543

By setting height to 200 you were causing images to stretch.  Setting it to a max-height, makes it fit auto resize to fit your max height.
@rodrigok
Copy link
Member

Doing that, messages start to move up and down until all images has loaded and their size was setted.

Another ideia keeping the height of image content fixed?

@rodrigok
Copy link
Member

Maybe:

<template name="oembedImageWidget">
    <a href="{{url}}" target="_blank">
        {{#if parsedUrl}}
            <blockquote>
                <div style="background-image: url({{url}}); background-size: contain; background-repeat: no-repeat; background-position: center left; height: 200px;" ></div>
            </blockquote>
        {{/if}}
    </a>
</template>

@geekgonecrazy
Copy link
Contributor Author

looks like just putting a div around it and doing:

<div style="height:200px;">
   <img src="{{url}}" style="max-width: 100%; max-height: 200px;" />
</div>

Might do the trick.

Are there performance gains from doing css side vs img tag?

@rodrigok
Copy link
Member

I didn't remember why we use img instead div with background. Do you remember @engelgabriel?

@geekgonecrazy
Copy link
Contributor Author

Just realized something. You'll probably want to stick to img. Especially if you do end up implementing #537

Could do something like:

<img data-src="{{url}}" style="max-width: 100%; max-height: 200px;" />

Then clicking what ever button you use you could do something like:

img.attr('src', img.data('src'));

Obviously there are other ways..

@rodrigok
Copy link
Member

I don't know, probably we will sove the #537 using a more complex html to render instead the image to show the file type and file size. We can do this changes in click event and changing some flag in message to force re render with the image visible.

This way area doesn't expand after the image loads causing content to shift.
@geekgonecrazy
Copy link
Contributor Author

Ah yeah. I've also gone similar routes on other projects.

Changed to a fixed height div. If css is more performant. I'll gladly make the change. Just trying to help make things look good :)

@rodrigok
Copy link
Member

I don't know about the performance impact but I prefer the my suggestion, looks more clean and keep image vertical centered if height is less than 200px.

Suggested by @rodrigok.

Looks much better on small images to be aligned in the center.  Especially if height needs to remain at 200px;
rodrigok added a commit that referenced this pull request Aug 22, 2015
@rodrigok rodrigok merged commit 719fb97 into RocketChat:master Aug 22, 2015
HappyTobi pushed a commit to HappyTobi/Rocket.Chat that referenced this pull request Jul 10, 2018
…cketChat#544)

* Adds extra option to trigger change in tray icon

When minimizing/restoring the window it doesn't fire the "open/close" events,
so if you minimize it and right-click the tray icon you will see "Hide" option,
clicking it won't change anything as the window is already hidden

* Removes console.log

* On clicking and window visible, should close it

if window is visible left-click should hide it
HappyTobi pushed a commit to HappyTobi/Rocket.Chat that referenced this pull request Jul 10, 2018
…cketChat#544)

* Adds extra option to trigger change in tray icon

When minimizing/restoring the window it doesn't fire the "open/close" events,
so if you minimize it and right-click the tray icon you will see "Hide" option,
clicking it won't change anything as the window is already hidden

* Removes console.log

* On clicking and window visible, should close it

if window is visible left-click should hide it
Peym4n pushed a commit to redlink-gmbh/Rocket.Chat that referenced this pull request Apr 4, 2019
…nection

Don't load the Smarti Widget if Smarti is disabled
Shailesh351 pushed a commit to Shailesh351/Rocket.Chat that referenced this pull request Apr 5, 2021
…cess

[NEW] Add feature to post process push message and display as a rich message
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.

2 participants