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.

Sign up for GitHub

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

Jump to bottom

Fix #175 : Add a docs section on how to change the font stack #212

Merged
merged 4 commits into from
Oct 24, 2021
Merged

Fix #175 : Add a docs section on how to change the font stack #212

merged 4 commits into from
Oct 24, 2021

Conversation

LoganTann
Copy link

@LoganTann LoganTann commented Oct 18, 2021 •
edited
Loading

Proposed changes

Fixes #175

  • add docs explaining how to change the font stack.
  • (There is also an alternative version, which is just restores a deleted part of the docs : see the 2th commit)
  • remove unused (commented) css code

Screenshots (if appropriate) or codepen:

image

Types of changes

  • Documentation update
  • Minor refactoring

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • (not required) I have added tests to cover my changes.
  • All new and existing tests passed.

@LoganTann
docs(typography): update how to change the font stack
4f6eb5c 
I'm still wondering why the font section have been deleted...
See : https://git.io/JK9is

Closes #175
@LoganTann
refactor(_global.scss): remove unused commented out code
fc01db1
@LoganTann
docs(typography): change the roboto section to the fontstack section
3548ebb 
I'm removing the most of the section about the Roboto Font by just
keeping the "how to change the font stack" part.

Fixes #175
@LoganTann
Copy link
Author

Whoo It took me so much time to figure out what was wrong with commitlint's errors.

Anyway, the fix for the issue #175 is ready to be reviewed.

@LoganTann LoganTann requested a review from a team October 18, 2021 11:36
roiLeo
roiLeo approved these changes Oct 18, 2021
View reviewed changes
Copy link
Member

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

LGTM

@RamonvdW
Copy link

RamonvdW commented Oct 18, 2021 •
edited
Loading

Looks like a good addition that points the developers to the correct statements to change.
One improvement could be to mention the full path to the source file to change as this requires a re-compile.
For new developers this might not be obvious. There is a section on that aspect, so maybe link to that as a further-read?

@LoganTann
Copy link
Author

Looks like a good addition that points the developers to the correct statements to change. One improvement could be to mention the full path to the source file to change as this requires a re-compile. For new developers this might not be obvious. There is a section on that aspect, so maybe link to that as a further-read?

I don't understand

In order to change the font stack, you just have to override the rules by copy/paste the snippet to your css, no need for patching materialize's stylesheet?

@RamonvdW
Copy link

Thank you for pointing that out. When I read the updated section it clearly states this.

@LoganTann
docs(typography): update description of the section "Changing the fon…
0def7b7 
…t stack"

this applies the suggested changed of this review :
#212 (comment)
@LoganTann LoganTann requested a review from a team October 21, 2021 06:52
@LoganTann
Copy link
Author

Any updates about my PR ?
It's ready to be merged.

@DanielRuf DanielRuf merged commit 489f023 into materializecss:main Oct 24, 2021
@LoganTann LoganTann deleted the issue175-fontStackDocs branch October 24, 2021 11:26
@Smankusors Smankusors added the documentation Improvements or additions to documentation label Apr 7, 2022
DyegoCosta pushed a commit to DyegoCosta/materialize that referenced this pull request Jun 27, 2022
@LoganTann @DyegoCosta
docs(typography): update description of the section "Changing the fon…
adee758 
…t stack"

this applies the suggested changed of this review :
materializecss#212 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRuf DanielRuf DanielRuf left review comments

@Smankusors Smankusors Smankusors left review comments

@roiLeo roiLeo roiLeo approved these changes

@RamonvdW RamonvdW RamonvdW approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Font-stack not documented and applied to few elements only
5 participants
@LoganTann @RamonvdW @DanielRuf @roiLeo @Smankusors