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.

Sign up for GitHub

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

Jump to bottom

Improve Add Covers as Comment #6046

Merged
merged 2 commits into from
Feb 4, 2022
Merged

Improve Add Covers as Comment #6046

merged 2 commits into from
Feb 4, 2022

Conversation

Yashs911
Copy link
Contributor

Related to #5716

I don't know why but on testing I can only see the covers changes done by me and not by anyone else.

I have tried to fix a few things which I noticed on testing.
Due to a PWA feature, any links with 404 image src will display OL Logo so adding height fixes the UI aspect

Screenshot

image

Stakeholders

@jimchamp

@Yashs911
Copy link
Contributor Author

Currently on testing
image

@jimchamp
Copy link
Collaborator

I noticed that dcapillae's image is present in the production book page, but not in testing. I'm wondering if he added the image in production. Did you add your cover images in testing?

Testing has some caching issues, which may be causing this. I'll investigate and let you know what I find out.

@jimchamp jimchamp self-assigned this Jan 12, 2022
@Yashs911
Copy link
Contributor Author

Did you add your cover images in testing?

Yes, can you try adding some covers in testing (I usually download the same cover and reupload it)

@jimchamp jimchamp added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Jan 24, 2022
@jimchamp jimchamp added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] Priority: 2 Important, as time permits. [managed] and removed Priority: 1 Do this week, receiving emails, time sensitive, . [managed] labels Feb 1, 2022
@Yashs911
Copy link
Contributor Author

Yashs911 commented Feb 3, 2022

@jimchamp Hey, I think after Mek's commit this PR is good to go

jimchamp
jimchamp approved these changes Feb 4, 2022
View reviewed changes
Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Thanks @Yashs911!

@jimchamp jimchamp merged commit feaf582 into internetarchive:master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimchamp jimchamp jimchamp approved these changes

Assignees

@jimchamp jimchamp

Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] Priority: 2 Important, as time permits. [managed]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Yashs911 @jimchamp @mekarpeles