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 lazy load was breaking attachments #10904

Merged
merged 3 commits into from May 29, 2018

Conversation

Projects
None yet
8 participants
@ggazzo
Copy link
Member

ggazzo commented May 28, 2018

List to check

  • Avatars

  • Attachments

Closes #10895

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10904 May 28, 2018 Inactive

@ggazzo ggazzo requested review from karlprieb and sampaiodiego May 28, 2018

element.className = element.className.replace('lazy-img', '');
element.src = src;
}
el.removeAttribute('data-src');

This comment has been minimized.

@sampaiodiego

sampaiodiego May 28, 2018

Member

change el to element

@JSzaszvari

This comment has been minimized.

Copy link
Member

JSzaszvari commented May 28, 2018

@ggazzo Not sure if this is related, but seeing this as well when running tests as well as my staging environment.

When searching for a user, a good majority of the time none of the avatars show up like this:

image

2018-05-29_00-15-53

Happy to log a separate bug report if you'd like, But not sure if it's related to this issue.

Not seeing anything in the console or logs, except for the initial click on the search button witch throws the following whenever it's clicked

index.js:40 Uncaught TypeError: el.getBoundingClientRect is not a function
    at index.js:40```

@ggazzo ggazzo temporarily deployed to rocket-chat-pr-10904 May 28, 2018 Inactive

@ggazzo ggazzo changed the title [FIX] Lazyload broken attachments [FIX] Lazyload broken attachments/ Broadcast private messages May 28, 2018

@ggazzo

This comment has been minimized.

Copy link
Member Author

ggazzo commented May 28, 2018

thanks @JSzaszvari I think its working now :x

@ggazzo

This comment has been minimized.

Copy link
Member Author

ggazzo commented May 28, 2018

<3

@tsukiRep

This comment has been minimized.

Copy link
Contributor

tsukiRep commented May 28, 2018

When can we expect this to be merged?

@JSzaszvari

This comment has been minimized.

Copy link
Member

JSzaszvari commented May 29, 2018

Amazing @ggazzo - Working great now

@vynmera

This comment has been minimized.

Copy link
Contributor

vynmera commented May 29, 2018

Please add this as hotfix, it is currently causing a lot of issues on my team (we have to resort to the insecure imgur now, or tell people to download the images)
Thank you!

@glebtv

This comment has been minimized.

Copy link

glebtv commented May 29, 2018

This is a user script to work around this issue:

setInterval(function() {
	document.querySelectorAll(".attachment-image img").forEach(function(v) { if (v.src != v.dataset.src) {v.src = v.dataset.src; } })
}, 1000)

@rodrigok rodrigok added this to the 0.65.1 milestone May 29, 2018

@ggazzo

This comment has been minimized.

Copy link
Member Author

ggazzo commented May 29, 2018

@glebtv you could remove the lazy-img class...

setInterval(function() {
	document.querySelectorAll(".attachment-image .lazy-img").forEach(function(el) { 
    	el.src = el.dataset.src;
		el.removeAttribute('data-src');
		el.className = el.className.replace('lazy-img', '');
	})
}, 1000)

but I think until the end of day it will be released

@rodrigok rodrigok changed the title [FIX] Lazyload broken attachments/ Broadcast private messages [FIX] Image lazy load was breaking attachments May 29, 2018

@rodrigok rodrigok merged commit c5c65bd into develop May 29, 2018

4 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test-with-oplog Your tests passed on CircleCI!
Details
ci/circleci: test-without-oplog Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@rodrigok rodrigok deleted the lazyload-fix-regression branch May 29, 2018

sampaiodiego added a commit that referenced this pull request May 30, 2018

Merge pull request #10904 from RocketChat/lazyload-fix-regression
[FIX] Image lazy load was breaking attachments

engelgabriel added a commit that referenced this pull request Jun 4, 2018

Merge branch 'master' into sync-master
* master:
  Bump version to 0.65.1
  Merge pull request #10940 from RocketChat/fix-livechat-not-loading
  Merge pull request #10934 from RocketChat/prevent-sending-exceptions-to-channel-before-startup
  Merge pull request #10928 from RocketChat/fix-email-notification-link
  Merge pull request #10904 from RocketChat/lazyload-fix-regression
  Merge pull request #10851 from RocketChat/leave-room-bug
  fix search shortcut text
  Display vertical scrollbar on demand

# Conflicts:
#	.docker/Dockerfile
#	.docker/Dockerfile.rhel
#	.sandstorm/sandstorm-pkgdef.capnp
#	.travis/snap.sh
#	HISTORY.md
#	package.json
#	packages/rocketchat-lib/rocketchat.info

@rodrigok rodrigok referenced this pull request Jun 28, 2018

Merged

Release 0.66.0 #11278

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.