Skip to content

change the layout for the Footer and style it#86

Closed
nahed2019 wants to merge 2 commits intoCode-Institute-Community:masterfrom
nahed2019:first-test
Closed

change the layout for the Footer and style it#86
nahed2019 wants to merge 2 commits intoCode-Institute-Community:masterfrom
nahed2019:first-test

Conversation

@nahed2019
Copy link

Description:

Just rearrange the layout of footer and style it.
this first try I just want to learn how we can contribute.

Related Issue:

Link to the related Issue

Configuration instructions:

List any non-trivial configuration instructions (if any)

**Testing:

image

Explain what kind of testing was performed and list any testing instructions (if needed)

Screenshots:

Add any screenshots (if needed)

Additional Information:

Any other information that is needed




footer h4 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would suggest to target with id or class instead of using elements like h4 if this changes to anything else the CSS breaks.

</div>
</div>

<hr>
Copy link
Contributor

Choose a reason for hiding this comment

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

<hr> are normally deprecated so would suggest using CSS to create the border

Copy link
Member

@stefdworschak stefdworschak left a comment

Choose a reason for hiding this comment

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

@nahed2019 could you make the changes Simen requested and tag me in this once done to approve it?

@nahed2019
Copy link
Author

nahed2019 commented Oct 20, 2020 via email

@auxfuse
Copy link
Contributor

auxfuse commented Oct 20, 2020

Can I ask why we are restyling the Footer that has already been a previously approved PR request and was structured inline with the CI template? The orange icons are garishly poor against the native grey background theme color and the way it was prior was much better in terms of UX to appeal to those users who may have a visual impairments especially around color contrast.

Is this not essentially a duplicate PR of works already commited and merged.

@Eventyret @stefdworschak @nahed2019 This PR request I am talking about btw was PR: #F01 Footer Content & Styling #53 #74

@nahed2019
Copy link
Author

nahed2019 commented Oct 20, 2020 via email

@stefdworschak
Copy link
Member

@nahed2019 sorry for only getting back to you on this now, but after some consideration here I would like to leave the design on the footer as it is. I would suggest that we close the PR and maybe you can focus on styling some of the pages used by all_auth (see screenshot).
image

@nahed2019
Copy link
Author

nahed2019 commented Oct 21, 2020 via email

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

Comments