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

some tags did not have closing tags and indentation was not proper in this file #2969

Merged
merged 3 commits into from
Feb 10, 2020

Conversation

aashimgarg
Copy link
Contributor

Issue solved #2798

Fixes HTML validation issues in
openlibrary/admin/templates/admin/index.html

Issues:

  1. In this index.html file the table data , strong , and br did not have closing tags.
  2. the indentation was not proper.

What is done:
1.Made the indentation proper. (as now it is much easier to understand)
2. gave the closing tags.

Stakeholders:
@koderjoker

@aashimgarg aashimgarg requested review from tfmorris and koderjoker and removed request for tfmorris February 4, 2020 14:34
@aashimgarg aashimgarg changed the title some tags did not have closing tags and indentation in this file some tags did not have closing tags and indentation was not proper in this file Feb 4, 2020
@tfmorris tfmorris removed their assignment Feb 4, 2020
@aashimgarg aashimgarg requested a review from jdlrobson February 5, 2020 21:23
@aashimgarg
Copy link
Contributor Author

@jdlrobson @koderjoker

@aashimgarg
Copy link
Contributor Author

@jdlrobson is it okay?

@jdlrobson
Copy link
Collaborator

jdlrobson commented Feb 10, 2020

I was hoping you could reassure me. Haven't looked at these pages before. :)

Can you post some screenshots?

@aashimgarg
Copy link
Contributor Author

aashimgarg commented Feb 10, 2020

These are the web.py templates for the frontend.
these are the tag errors that i corrected and made a pull request for this.

@jdlrobson
Copy link
Collaborator

I understand what you are doing.

What I don't understand is what the content after "Prolific Member Editors" does and why you need to change the markup there.

Are you able to get this to display locally?
If so can you post a screenshot of how this renders before and after?

@aashimgarg
Copy link
Contributor Author

aashimgarg commented Feb 10, 2020

I changed the markup because as href attribute of the anchor tag in html --> href="_" encloses the link in the double codes.

@jdlrobson
Copy link
Collaborator

Please post a screenshot.

@hornc hornc self-assigned this Feb 10, 2020
@hornc
Copy link
Collaborator

hornc commented Feb 10, 2020

Thanks @aashimgarg

@hornc hornc merged commit 3d83f08 into internetarchive:master Feb 10, 2020
@hornc
Copy link
Collaborator

hornc commented Feb 11, 2020

@aashimgarg @jdlrobson I chose to merge this without the screenshots as this is dead-code (unfortunately) and I don't believe you could view this page properly with the current site config.

@jdlrobson was right to ask for screenshots, testing page changes in your local env is always the right thing to do, and we all would have noticed sooner that this page isn't used and can't be viewed :)

I will be adding an issue to investigate some of these unused admin templates so they can be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants