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

Fix validation errors on homepage #4481

Merged
merged 5 commits into from
Jan 29, 2021
Merged

Fix validation errors on homepage #4481

merged 5 commits into from
Jan 29, 2021

Conversation

SaravgiYash
Copy link
Contributor

@SaravgiYash SaravgiYash commented Jan 24, 2021 •
edited
Loading

Closes #4441

@jdlrobson, I don't know why it's not working on the local env. Can you please review and guide me what all changes do I need to make?

Stakeholders

@jdlrobson

@jdlrobson jdlrobson self-assigned this Jan 24, 2021
@SaravgiYash
Copy link
Contributor Author

@jdlrobson please test these changes because on local env I was getting "Unable to render this page" error

@SaravgiYash
Copy link
Contributor Author

@jdlrobson I have made few changes based on your suggestions, and this seems to work
image

jdlrobson
jdlrobson reviewed Jan 27, 2021
View reviewed changes
Copy link
Collaborator

@jdlrobson jdlrobson left a comment

Choose a reason for hiding this comment

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

Thoughts?

openlibrary/templates/books/custom_carousel.html Outdated Show resolved Hide resolved
@SaravgiYash
Copy link
Contributor Author

SaravgiYash commented Jan 27, 2021 •
edited
Loading

@jdlrobson What about if we set the img_attr to src by default and a single if statement for lazy type:

$ img_attr = 'src'
$if lazy:
    $ img_attr = 'data-lazy'
    $ data_url = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII='
    $ src_attr = 'src'

Will this work?
EDIT: Bad Idea

@SaravgiYash
Copy link
Contributor Author

@jdlrobson I tested this locally, and its shows 0 validation errors.
I just hope it doesn't affect image loading performance.
and sorry for any inconvenience caused due to this PR :)

@SaravgiYash SaravgiYash mentioned this pull request Jan 27, 2021
Closed
@jdlrobson jdlrobson merged commit af3e69a into internetarchive:master Jan 29, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
bd55d46 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
5d4852d 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
74c0de8 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
d6b669b 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
2e2c4db 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 15, 2021
@SaravgiYash @Sabreen-Parveen
Lazy loaded carousel items should have a src attribute
47301a1 
A 1px by 1px base64 encoded thumbnail is used for covers that will
be lazy loaded as a placeholder. The data-src attribute points to the
real cover and is only added when necessary.

This resolves HTML validation issues on the home page.

Fixes: internetarchive#4481
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdlrobson jdlrobson jdlrobson requested changes

Assignees

@jdlrobson jdlrobson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix validation issues on the home page
2 participants
@SaravgiYash @jdlrobson