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

[material-ui] Refine Blog template #42825

Merged
merged 89 commits into from
Aug 2, 2024
Merged

[material-ui] Refine Blog template #42825

merged 89 commits into from
Aug 2, 2024

Conversation

zanivan
Copy link
Contributor

@zanivan zanivan commented Jul 2, 2024

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 23, 2024
@zanivan
Copy link
Contributor Author

zanivan commented Jul 23, 2024

@DiegoAndai could you help me with failing tests? After fixing it, I believe this is ready for review :)

test_lint seems to be failing on this file, but I don't know exactly what it does.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 23, 2024
@DiegoAndai
Copy link
Member

DiegoAndai commented Jul 25, 2024

Hey @oliviertassinari! This template no longer uses markdown, so docs/data/material/getting-started/templates/blog/Markdown.tsx (and .js) were removed. We have a test that uses it here: https://github.com/mui/material-ui/blob/next/benchmark/server/scenarios/docs.js

Can we remove it from the benchmark test? Asking as you initially introduced the test (PR):

diff --git a/benchmark/server/scenarios/docs.js b/benchmark/server/scenarios/docs.js
index 25fa13feb8..a26b6d85cf 100644
--- a/benchmark/server/scenarios/docs.js
+++ b/benchmark/server/scenarios/docs.js
@@ -5,7 +5,6 @@ import Benchmark from 'benchmark';
 import * as React from 'react';
 import * as ReactDOMServer from 'react-dom/server';
 import { MarkdownElement } from '@mui/docs/MarkdownElement';
-import Markdown from 'docs/data/material/getting-started/templates/blog/Markdown';
 import { createStore } from 'redux';
 import { Provider } from 'react-redux';
 
@@ -28,9 +27,6 @@ const store = createStore((state) => state, {
 });
 
 suite
-  .add('Markdown', () => {
-    ReactDOMServer.renderToString(<Markdown>{markdown}</Markdown>);
-  })
   .add('MarkdownElement', () => {
     ReactDOMServer.renderToString(
       <Provider store={store}>

@zanivan zanivan changed the title [draft][material-ui] Refine Blog template [material-ui] Refine Blog template Jul 30, 2024
@zanivan zanivan marked this pull request as ready for review July 30, 2024 14:02
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Looks good! Just have some formatting nits 😊

@zanivan
Copy link
Contributor Author

zanivan commented Aug 2, 2024

@aarongarciah I forgot you were on PTO this week, so I'm merging this one to start working on unifying the themes and making it consistent. Feel free to add your feedback here or in an issue, and I'll address it in a future PR 👍

@zanivan zanivan merged commit 5f202fd into next Aug 2, 2024
28 checks passed
@zanivan zanivan deleted the material-ui-template-blog branch August 2, 2024 11:54
@aarongarciah
Copy link
Member

Great work @zanivan. I didn't review the PR in depth, but a couple of details caught my eye:

  • It took me a while until I realized there was a theme switcher at the bottom.
  • The padding on the chips looks too tight.
  • The hover state on the chips maybe is a bit too subtle.
  • The hover state with the underline effect feels slow on the "Latest" articles. Maybe we could explore another effect e.g. underline all of the text rows in case of multiline title and use a fade-in effect instead of animating the underling. The underline taking the whole block widths feels a bit weird.
Screenshot 2024-08-05 at 14 43 46 Screenshot 2024-08-05 at 14 48 03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This is about UI or UX design, please involve a designer docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants