fix app thumbnail viewing on spee.ch #513

2018-07-08T01:28:44+02:00

bones7242 commented

2018-07-08 01:28:44 +02:00

(Migrated from github.com)

This PR fixes the issue where spee.ch was showing a page with a broken link instead of the asset. This issues seems to have been a result of cloudflare caching the page document and serving that instead of the image or video.

added check for file name in publish request
fixed accuracy of some file check error messages
remove default content type of image/jpeg on file serving
changed embeded assets to use the spee.ch/asset/<name>/<claimid> route which is specific for serving the asset itself, thus avoiding the confusion which lead to cloudflare serving the document from the cache

This PR fixes the issue where spee.ch was showing a page with a broken link instead of the asset. This issues seems to have been a result of cloudflare caching the page document and serving that instead of the image or video. - added check for file name in publish request - fixed accuracy of some file check error messages - remove default content type of `image/jpeg` on file serving - changed embeded assets to use the `spee.ch/asset/<name>/<claimid>` route which is specific for serving the asset itself, thus avoiding the confusion which lead to cloudflare serving the document from the cache

skhameneh (Migrated from github.com) approved these changes 2018-07-11 19:11:13 +02:00

client/build/components/AssetPreview/index.js

					
				@ -21,3 +21,3 @@

				      thumbnail = _ref$claimData.thumbnail;

				  var directSourceLink = "".concat(claimId, "/").concat(name, ".").concat(fileExt);

				  var directSourceLink = "asset/".concat(name, "/").concat(claimId);

				  var showUrlLink = "/".concat(claimId, "/").concat(name);

skhameneh (Migrated from github.com) commented

2018-07-11 19:05:38 +02:00

not a stopper, but it would be ideal to stick with single or double quotes, I see some mixed formatting

client/build/containers/AssetDisplay/view.js

					
				@ -84,3 +84,3 @@

				              className: "asset-image",

				              src: "/".concat(claimId, "/").concat(name, ".").concat(fileExt),

				              src: "/asset/".concat(name, "/").concat(claimId),

				              alt: name

skhameneh (Migrated from github.com) commented

2018-07-11 19:07:55 +02:00

Using concat on strings is a little different that common convention, does it have a negative performance impact?
You could also use template literals

src: `/asset/${name}/${claimId}`

Using concat on strings is a little different that common convention, does it have a negative performance impact? You could also use template literals ``` src: `/asset/${name}/${claimId}`

kauffj (Migrated from github.com) reviewed 2018-07-11 19:41:28 +02:00

client/build/components/AssetPreview/index.js

					
				@ -21,3 +21,3 @@

				      thumbnail = _ref$claimData.thumbnail;

				  var directSourceLink = "".concat(claimId, "/").concat(name, ".").concat(fileExt);

				  var directSourceLink = "asset/".concat(name, "/").concat(claimId);

				  var showUrlLink = "/".concat(claimId, "/").concat(name);

kauffj (Migrated from github.com) commented

2018-07-11 19:41:27 +02:00

@billbitt I saw #405 was closed but if you add Prettier I believe this will never be an issue again.

bones7242 (Migrated from github.com) reviewed 2018-07-11 19:56:06 +02:00

client/build/components/AssetPreview/index.js

					
				@ -21,3 +21,3 @@

				      thumbnail = _ref$claimData.thumbnail;

				  var directSourceLink = "".concat(claimId, "/").concat(name, ".").concat(fileExt);

				  var directSourceLink = "asset/".concat(name, "/").concat(claimId);

				  var showUrlLink = "/".concat(claimId, "/").concat(name);

bones7242 (Migrated from github.com) commented

2018-07-11 19:56:06 +02:00

We should add Prettier (I reopened #405), but these files are the build result from babel transpiling the es6 code.
I'd like to avoid checking the built code into version control, but we have to include it at this point so that it can be imported by other packages like lbryio/www.spee.ch. I'd like merge lbryio/www.spee.ch and lbryio/spee.ch (#516) which should result in being able to have one build process that does not need to check built code into version control.

We should add Prettier (I reopened #405), but these files are the build result from babel transpiling the es6 code. I'd like to avoid checking the built code into version control, but we have to include it at this point so that it can be imported by other packages like `lbryio/www.spee.ch`. I'd like merge `lbryio/www.spee.ch` and `lbryio/spee.ch` (#516) which should result in being able to have one build process that does not need to check built code into version control.

bones7242 (Migrated from github.com) reviewed 2018-07-11 19:56:35 +02:00

client/build/containers/AssetDisplay/view.js

					
				@ -84,3 +84,3 @@

				              className: "asset-image",

				              src: "/".concat(claimId, "/").concat(name, ".").concat(fileExt),

				              src: "/asset/".concat(name, "/").concat(claimId),

				              alt: name

bones7242 (Migrated from github.com) commented

2018-07-11 19:56:35 +02:00

see above, but this is the result of babel

Sign in to join this conversation.

No reviewers

skhameneh

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/spee.ch#513

Rows
Columns