Skip to content
This repository was archived by the owner on Mar 7, 2024. It is now read-only.

Add Montserrat Font #57

Merged
merged 3 commits into from
Sep 21, 2020
Merged

Add Montserrat Font #57

merged 3 commits into from
Sep 21, 2020

Conversation

Abhishek-kumar09
Copy link
Contributor

Closes #11

@Abhishek-kumar09
Copy link
Contributor Author

Abhishek-kumar09 commented Aug 9, 2020 •
edited
Loading

@KeenWarrior @GangaChatrvedi
I have added montserrat to every Typography, I didn't found any other good way...There are no css files dedicated to these.
Also, There are lots of extra(bulky) code written which is not handy or may be difficult to understand for new contributors, I think!

@Abhishek-kumar09 Abhishek-kumar09 changed the title Css fix Add Montserrat Font Aug 9, 2020
@GangaChatrvedi
Copy link
Member

GangaChatrvedi commented Aug 12, 2020 •
edited
Loading

@Abhishek-kumar09 Take a look at this.

And We'll be cleaning up the bulky code once the team gets free as everybody now has so much to do on priority.

@Abhishek-kumar09
Copy link
Contributor Author

@Abhishek-kumar09 Take a look at this.

Thanks, great help.

KeenWarrior
KeenWarrior suggested changes Sep 21, 2020
View reviewed changes
src/theme/typography.js
@@ -1,8 +1,17 @@
export default {
fontFamily: [
'"Montserrat"',
Copy link
Contributor

Choose a reason for hiding this comment

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

just keep Montserrat and remove others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the Monteserrat font is not fetched somehow, than It will show the very basic Aerial(I guess). So atleast two fonts are required I think.
Else it's your choice. I would remove the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/layouts/MainLayout/TopBar/index.js Outdated
@@ -140,7 +140,6 @@ function TopBar({ className, onMobileNavOpen, ...rest }) {
<Box ml={2}>
<OpenSource />
</Box>
<Box ml={2} flexGrow={0.05} />
Copy link
Contributor

Choose a reason for hiding this comment

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

revert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/components/HeaderItems.js Outdated
@@ -9,7 +9,7 @@ const useStyles = makeStyles(() => ({
position: 'relative',
fontColor: 'blue',
display: 'flex',
padding: ' 19px 19px',
padding: ' 19px 10px',
Copy link
Contributor

Choose a reason for hiding this comment

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

revert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@KeenWarrior
Copy link
Contributor

@Abhishek-kumar09 Please rebase and make the above changes.

Then We can continue to find where the fonts still face issues in the sandbox.

@Abhishek-kumar09 Abhishek-kumar09 force-pushed the css_fix branch 2 times, most recently from 0096ede to 3a308bf Compare September 21, 2020 13:13
@KeenWarrior KeenWarrior merged commit d413a6e into codeforcauseorg:development Sep 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KeenWarrior KeenWarrior Awaiting requested review from KeenWarrior

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update font family
3 participants
@Abhishek-kumar09 @GangaChatrvedi @KeenWarrior