Skip to content

Conversation

@zanivan
Copy link
Collaborator

@zanivan zanivan commented Jun 9, 2023

Part of #37555

The idea is to transform the existing Album template into a Landing Page template. It's part of our initiative to review and enhance the free templates we offer.

Current New
image image

👉 https://deploy-preview-37557--material-ui.netlify.app/material-ui/getting-started/templates/landing-page/

@zanivan zanivan added design: material This is about Material Design, please involve a visual or UX designer in the process package: material-ui labels Jun 9, 2023
@mui-bot
Copy link

mui-bot commented Jun 9, 2023

@zanivan zanivan self-assigned this Dec 4, 2023
@zanivan zanivan added design This is about UI or UX design, please involve a designer. docs Improvements or additions to the documentation. and removed design: material This is about Material Design, please involve a visual or UX designer in the process labels Dec 4, 2023
@mbrookes

This comment was marked as outdated.

@danilo-leal danilo-leal changed the title [template][material] Revamp album layout template [material-ui] Revamp the Album template design Dec 12, 2023
@zanivan

This comment was marked as outdated.

@zanivan zanivan requested a review from siriwatknp January 30, 2024 11:20
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.

Overall looks good. I just have a tiny design feedback

I think we shouldn't add the theme switch in the layout, I would rather have it be a floating element on the bottom right and a toggle button instead of a switch.

Screenshot 2024-01-30 at 19 06 08

I think that would make it easier to remove, and its use clearer.

@siriwatknp
Copy link
Member

siriwatknp commented Feb 1, 2024

@zanivan For non-component files, e.g. theme, you will need to exclude them from the regression test, see 11d46c7.

Comment on lines +66 to +71
ToggleCustomTheme.propTypes = {
showCustomTheme: PropTypes.shape({
valueOf: PropTypes.func.isRequired,
}).isRequired,
toggleCustomTheme: PropTypes.func.isRequired,
};
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove the proptypes? It's not for everyone especially people who use TypeScript.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's from docs:typescript:formatted, isn't it? Also, this is in the js. file, users who use Typescript will use the .tsx file, won't they?

Anyway, I leave the decision to you @siriwatknp :)

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 This is awesome! left one minor comment regarding proptypes.

Copy link
Collaborator

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Woohoo, I think we're good to go — any further tweaks can come iteratively from this point on. Let's 🚢 it; fantastic work!

@zanivan zanivan merged commit 0766bde into mui:master Feb 5, 2024
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Feb 6, 2024
Co-authored-by: Diego Andai <[email protected]>
Co-authored-by: Danilo Leal <[email protected]>
Co-authored-by: siriwatknp <[email protected]>
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants