Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Configure footer as slim with edit link #172

Merged
merged 3 commits into from
Jul 2, 2020
Merged

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Jul 1, 2020

Closes #167

This pull request seeks to add an "Edit this page" link to the footer of each page, modeled after the TTS Handbook.

Screenshots:

Before After
Before screenshot After screenshot

Reference:

Reference screenshot

As separate tasks, it may be worth considering:

  • Aligning on whether a "Return to top" link should be present
  • Aligning on whether a "File an issue" link should be present

@aduth
Copy link
Contributor Author

aduth commented Jul 1, 2020

🤔 Not sure on the build error:

- ./_site/accessibility-scanning/index.html
 
  *  linking to internal hash # that does not exist (line 415)
 
     <a href="#">Return to top</a>

It doesn't seem this change should have had any impact. The footer was updated to use the slim configuration, but as best I can tell, the markup produced should be the same:

@aduth
Copy link
Contributor Author

aduth commented Jul 1, 2020

Still not sure why it would suddenly become a failure as of this pull request, but possible resolutions could include:

@ryanhofdotgov
Copy link
Contributor

Dug up a recent-ish PR that fixed similar problem: #130
"[added] data-proofer-ignore attribute to the link"

@aduth
Copy link
Contributor Author

aduth commented Jul 1, 2020

@ryanhofdotgov Ah! I guess a custom footer--medium.html was implemented to overwrite the theme default. I'll plan to compare it to the default to see if there's any other differences to note. The file should probably be removed, or an equivalent one made for the slim style.

@aduth
Copy link
Contributor Author

aduth commented Jul 2, 2020

Here's the diff between the override and the base theme:

14c14
<       <a href="{{ footer.top.href | default: '#' }}" data-proofer-ignore>{{ footer.top.text | default: 'Return to top' }}</a>
---
>       <a href="{{ footer.top.href | default: '#' }}">{{ footer.top.text | default: 'Return to top' }}</a>
42c42
<   {% if footer.logos or footer.heading or footer.contact %}
---
>   {% if footer.logos or footer.heading or footer.contact or footer.edit_page %}
61a62,63
> 
>       {% if footer.contact %}
63,67c65,93
<         <ul class="usa-unstyled-list">
<           <li><a href="https://github.com/18F/uswds-jekyll">18F USWDS-Jekyll theme</a></li>
<           <li>Hosted by <a href="https://federalist.18f.gov/">Federalist</a></li>
<           <li>Maintained by <a href="https://18f.gsa.gov/">18F</a></li>
<         </ul>
---
>         {% assign social_links = site.data.footer.contact.social_links %}
>         {% if footer.contact.contact_links %}
>           {% for _link in social_links %}
>             <a class="usa-link-{{ _link.type | default: 'generic' }}" href="{% if _link.external == true %}{{ _link.href }}{% else %}{{ _link.href | relative_url }}{% endif %}">
>               <span>{{ _link.text }}</span>
>             </a>
>           {% endfor %}
>         {% endif %}
>         {% if footer.contact.heading %}
>           <h3 class="usa-footer-contact-heading">{{ footer.contact.heading }}</h3>
>           {% endif %}
>           {% if footer.contact.contact_links %}
>             <address>
>               {% assign contact_links = site.data.footer.contact.contact_links %}
>               {% for _link in contact_links %}
>                 <div class="usa-footer-primary-content usa-footer-contact_info">
>                   <p>
>                     <a href="{% if _link.external == true %}{{ _link.href }}{% else %}{{ _link.href | relative_url }}{% endif %}">
>                       {{ _link.text }}
>                     </a>
>                   </p>
>                 </div>
>               {% endfor %}
>             </address>
>         {% endif %}
>       {% endif %}
>       {% if footer.edit_page %}
>         {% include components/github-edit.html footer=footer path=page.path %}
>       {% endif %}

Some of it is explained by customizations (#130) and upstream updates (18F/uswds-jekyll@ce5ded4). It was originally added in 18F/frontend#208, but I can't tell what context there'd have been for overriding the base file.

I think if the goal is to align to the TTS Handbook, it shouldn't be much a concern to lose these navigation links customizations.

For the purpose of getting the build to pass, I could introduce an override file for the slim footer, with a similar fix as what was done in #130. I suspect the options in #172 (comment) would probably be more durable / maintainable. I'm leaning toward removing the "Return to top" link.

@aduth
Copy link
Contributor Author

aduth commented Jul 2, 2020

Relevant for the errors previously flagged by html-proofer: 18F/uswds-jekyll#191

@ryanhofdotgov
Copy link
Contributor

Sounds reasonable to me – thanks! I think a lot of the little inconsistencies in this guide from years of divergence will get solved by just wholesale adopting most of the guide setup from one of the other repos and making that the reusable thing across most guides. Here's the most recent comprehensive discussion on that topic:
https://docs.google.com/document/d/1LCKZKqoEEYOCwntZpTbHp8bW7tdnPnmmKVJ-YNHAUy4/edit

@ryanhofdotgov ryanhofdotgov self-requested a review July 2, 2020 16:36
@aduth aduth merged commit 3f12518 into master Jul 2, 2020
@aduth aduth deleted the update/footer-edit-link branch July 2, 2020 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add links to "edit this page"
2 participants