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

Update footer design #2495

Merged
merged 3 commits into from
May 24, 2021
Merged

Update footer design #2495

merged 3 commits into from
May 24, 2021

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented May 21, 2021

  • deletes IconSet component(As far as I can tell it is only used in the footer component, so I went ahead and deleted it and used inline svg with the IconSet component).
  • adds youtube, linkedin, and discord icons to IconSet component(I reused the github and twitter svgs that were in the IconSet component. The only difference as far as I can tell is that the github is smaller.)
  • adds cml, studio, and iterative svg images to img folder
  • updated img/logo.svg(As far as I can tell, the logo.svg is only used in the nav bar and the footer.)

* delete IconSet component
* add youtube, linkedin, and discord icons to IconSet component
* add cml, studio, and iterative svg images to img
* update img/logo.svg
@shcheklein shcheklein temporarily deployed to dvc-org-update-footer-d-ys94h8 May 21, 2021 20:21 Inactive

.companyDescription {
@media (--xs-scr) {
font-size: 14px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I did with cml.dev and iterative.ai, I increased the company description from 12px to 14px
image

@mixin active;
@mixin hover;

font-weight: 800;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While the font-weight says 800, I'm pretty sure it's 600 since that appears to be the largest weight our font has:
image

@julieg18
Copy link
Contributor Author

To fix the codeclimate issues, I can place all of the Footer link info into a js array and map over it in the footer JSX.

@rogermparent
Copy link
Contributor

To fix the codeclimate issues, I can place all of the Footer link info into a js array and map over it in the footer JSX.

Sounds like a good idea to me!

@jorgeorpinel
Copy link
Contributor

(Assigned you for visibility in https://github.com/orgs/iterative/projects/119 @julieg18, thanks)

@julieg18 julieg18 temporarily deployed to dvc-org-update-footer-d-ys94h8 May 23, 2021 21:41 Inactive
@julieg18 julieg18 temporarily deployed to dvc-org-update-footer-d-ys94h8 May 23, 2021 21:52 Inactive
import styles from './styles.module.css'

const docsPage = getFirstPage()

const LayoutFooter: React.FC<Required<ILayoutModifiable>> = ({ modifiers }) => (
interface IFooterLinkData {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty new to TypeScript, please let me know if my typescript code can be improved!

}
]

const FooterLists: React.FC = () => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I move these functions(FooterLists and FooterSocialIcons) to a folder inside LayoutFooter?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like a sensible refactor, but not necessary before merge at least.

@rogermparent
Copy link
Contributor

Looks great, nice work!

@rogermparent rogermparent merged commit 9509abb into master May 24, 2021
@casperdcl casperdcl deleted the update-footer-design branch July 14, 2021 08:59
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.

None yet

4 participants