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

[next] Item 2 and 4 related to /styles/global.css of Issue #1548 #1562

Merged
merged 4 commits into from
Jan 20, 2021

Conversation

PedroFonsecaDEV
Copy link
Contributor

@PedroFonsecaDEV PedroFonsecaDEV commented Jan 19, 2021

Issue This PR Addresses

This PR solves item 2 and 4 of the issue: #1548

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

This PR solves item 2 and 4 of the issue: #1548

Regards item 2:
This CSS is coming from PageBase
Not sure if it needed to set

padding: none;
margin: none;

I think it's a good idea.
@tonyvugithub Do you know if MUI has something related to resetting padding or margin?

Regards item 4:
#1504 (comment)

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)

@PedroFonsecaDEV PedroFonsecaDEV added the area: nextjs Nextjs related issues label Jan 19, 2021
@PedroFonsecaDEV PedroFonsecaDEV added this to the 1.5 Release milestone Jan 19, 2021
@PedroFonsecaDEV PedroFonsecaDEV self-assigned this Jan 19, 2021
@PedroFonsecaDEV PedroFonsecaDEV changed the title [next] Item 3 (/styles/global.css) of Issue #1548 [next] Item 3 and 4 related to /styles/global.css of Issue #1548 Jan 19, 2021
@PedroFonsecaDEV PedroFonsecaDEV changed the title [next] Item 3 and 4 related to /styles/global.css of Issue #1548 [next] Item 3 and 5 related to /styles/global.css of Issue #1548 Jan 19, 2021
@tonyvugithub
Copy link
Contributor

You can set global style in MUI too. We can either do Next global styling or put everything in MUi

@tonyvugithub
Copy link
Contributor

tonyvugithub commented Jan 19, 2021

@PedroFonsecaDEV : Here. https://material-ui.com/customization/globals/. But putting all of global.css content to MUI will make the theming too big. Maybe just some of the properties.


/* -------------------------------------------------- */

/* telescope-post-content.css */
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing this inside global.css, can you move this to its own file: src/frontend/next/src/styles/telescope-post-content.css and do an @import here to include it.

We need to do the same with the highlight-js CSS stylesheet for our code formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor Author

@PedroFonsecaDEV PedroFonsecaDEV Jan 20, 2021

Choose a reason for hiding this comment

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

@humphd I could not find the file highlight-js. Could you please provide the path to me?

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to install the highlightjs package to next/, then pull the CSS file from node_modules:

    "highlight.js": "10.4.1",

Then we can use node_modules/highlight.js/styles/github.css

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Let's do it in a follow-up.

@PedroFonsecaDEV PedroFonsecaDEV changed the title [next] Item 3 and 5 related to /styles/global.css of Issue #1548 [next] Item 2 and 4 related to /styles/global.css of Issue #1548 Jan 20, 2021
@humphd
Copy link
Contributor

humphd commented Jan 20, 2021

This is ready to go, @PedroFonsecaDEV, land when ready.

@PedroFonsecaDEV PedroFonsecaDEV merged commit d869727 into Seneca-CDOT:master Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nextjs Nextjs related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants