[TAS-1216] ✨ Define book cover for PDF file #457
Conversation
AuroraHuang22
force-pushed
the
feature/pdf
branch
from
e3c74aa
to
a9fc7d9
Compare
AuroraHuang22
force-pushed
the
feature/pdf
branch
from
d8660fc
to
99fa573
Compare
components/IscnUploadForm.vue
Outdated
| for (const file of this.fileRecords) { | ||
| if (file.fileType === 'application/epub+zip') { | ||
| return false; | ||
| } | ||
| if (file.fileType === 'application/pdf') { | ||
| hasPDF = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Use .find seems to be simpler, for of is required when the iterable is async
There was a problem hiding this comment.
components/IscnUploadForm.vue
Outdated
| addToEpubMetadataList(ipfsHash: string, arweaveId: string) { | ||
| this.epubMetadataList.push({ | ||
| thumbnailIpfsHash: ipfsHash, | ||
| thumbnailArweaveId: arweaveId, | ||
| }) | ||
| } |
There was a problem hiding this comment.
This function doesn't seems necessary if it only has one line of content
There was a problem hiding this comment.
components/IscnUploadForm.vue
Outdated
| if ( | ||
| this.epubMetadataList[0] && | ||
| this.epubMetadataList[0].thumbnailArweaveId | ||
| ) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Does this mean if any thumbnailArweaveId exists for this.epubMetadataList we should return? If yes use .find
There was a problem hiding this comment.
components/IscnUploadForm.vue
Outdated
| const transactionHash = | ||
| // eslint-disable-next-line no-await-in-loop | ||
| existingData.transactionHash || (await this.sendArweaveFeeTx(file)) |
There was a problem hiding this comment.
Prefer explicit if block if we are calling a function in the fallback case
There was a problem hiding this comment.
components/IscnUploadForm.vue
Outdated
| return | ||
| } | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| for (const file of this.fileRecords) { |
There was a problem hiding this comment.
Prefer simple for loop if iterable is not async
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
components/IscnUploadForm.vue
Outdated
| @@ -837,6 +913,11 @@ export default class IscnUploadForm extends Vue { | |||
|
|
|||
| try { | |||
| this.uploadStatus = 'uploading'; | |||
|
|
|||
| if (this.checkUploadFileTypeIsPDF()) { | |||
There was a problem hiding this comment.
It seems the function does not simply check is PDF, but PDF and not EPUB, may be split 2 logic?
Sth like this.checkUploadFileTypeContainsPDF() && !this.checkUploadFileTypeContainsEPUB() (actually simple .find would do, we might not even need a function`
There was a problem hiding this comment.
AuroraHuang22
force-pushed
the
feature/pdf
branch
from
9cfa8a3
to
bc8ddb7
Compare
When a user uploads a PDF and a JPG/PNG image simultaneously, the first uploaded image should be set as the default cover image and written into the ISCN's thumbnail field.
QA flow: