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

Fixes 817 added Footer to DesktopHeader and MobileHeader #964

Merged
merged 3 commits into from
Apr 12, 2020

Conversation

cindyorangis
Copy link
Contributor

Issue This PR Addresses

Fixes #817

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Initially meant to put this Footer underneath all the blog posts but I notice that after we hit the last blog post, it actually continues to load more blog posts so it doesn't make sense to put the footer there. This adds Footers to the bottom of the side drawer for both the DesktopHeader and MobileHeader.

Annotation 2020-04-08 194402

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@cindyorangis cindyorangis self-assigned this Apr 8, 2020
@vercel vercel bot requested a deployment to Preview April 8, 2020 23:45 Abandoned
@vercel
Copy link

vercel bot commented Apr 8, 2020

Deployment failed with the following error:

request to https://api.zeit.co/v10/now/deployments?skipAutoDetectionConfirmation=1 failed, reason: socket hang up

@humphd
Copy link
Contributor

humphd commented Apr 9, 2020

Merge conflict here, let's fix that before we review.

@agarcia-caicedo
Copy link
Contributor

Do we want a link for the CDOT website on the footer?

@agarcia-caicedo
Copy link
Contributor

agarcia-caicedo commented Apr 11, 2020

Code wise I think it's fine, however because of #962, where would we put the footer when the drawer is removed?

raygervais
raygervais previously approved these changes Apr 11, 2020
Copy link
Contributor

@raygervais raygervais left a comment

Choose a reason for hiding this comment

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

Code wise, this looks good to me. The sidebar is shaping up great.

@cindyorangis
Copy link
Contributor Author

Do we want a link for the CDOT website on the footer?

Oh this is a good idea!

@cindyorangis
Copy link
Contributor Author

Code wise I think it's fine, however because of #962, where would we put the footer when the drawer is removed?

Thinking ahead, we should keep the drawer because after I finish #290 , I wanna add another tab that says DOCS underneath ABOUT. But #290 will probably done in May...

This is up for discussion. Do we wanna remove the drawer for a month and reintroduce it after? Or just keep it? There are plenty of new pages we can add in the future

@cindyorangis
Copy link
Contributor Author

added link to Seneca CDOT website :)

@cindyorangis cindyorangis requested a review from raygervais April 12, 2020 16:19
@cindyorangis cindyorangis merged commit fb41114 into Seneca-CDOT:master Apr 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Footer Improvements
4 participants