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

[website] Add Marblism diamond sponsor #41097

Merged
merged 51 commits into from Feb 19, 2024

Conversation

@rluzists1 rluzists1 added the website Pages that are not documentation-related, marketing-focused. label Feb 14, 2024
@rluzists1 rluzists1 requested a review from a team February 14, 2024 15:24
@mui-bot
Copy link

mui-bot commented Feb 14, 2024

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against d90fa32

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 14, 2024

@rluzists1 Please don't use @core, this notifies everyone and subscribe them to future notifications on this PR.

Instead, let's rely on the people responsible or accountable of https://www.notion.so/mui-org/Keep-sponsors-up-to-date-7ebff6991f2946ca853b0e1d3df3a826.

@oliviertassinari oliviertassinari removed the request for review from a team February 14, 2024 15:43
@danilo-leal
Copy link
Contributor

Please don't use @ core, this notifies everyone.

Quick aside about this: I've added some brief review "etiquette" instructions (that can be enhanced/beefed up) on the Notion GitHub PRs page. I think we can help folks who aren't that familiar with GitHub by writing down more of these "how-to-use" guides about the tools we use! @rluzists1 I wouldn't be surprised if you have just learned about this little instruction now through trial and error, but maybe for future non-devs interacting with code, this can be handy! :)

@rluzists1
Copy link
Contributor Author

I wouldn't be surprised if you have just learned about this little instruction now through trial and error, but maybe for future non-devs interacting with code, this can be handy! :)

Thanks @danilo-leal :) I'd used the @core for my career page PR so just assumed that was custom; agree having these how-tos is really helpful

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 14, 2024

I pushed a fix for a two-year-old bug: #33528 (comment), and increased the pixel ratio to support mobile (usually x3) for the exiting sponsors.

I didn't touch the new one (maybe I should have done this in a separate PR). The image handling of Marblism needs to be fixed though:

  • Missing dark mode
  • Pixelized rendering

@rluzists1
Copy link
Contributor Author

Applied the same changes to the marblism image on backers and READ.me

@oliviertassinari
Copy link
Member

Good on my end. I polished the existing sponsors as much as I could

@oliviertassinari oliviertassinari requested review from danilo-leal and removed request for samuelsycamore February 17, 2024 13:28
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 19, 2024
@rluzists1
Copy link
Contributor Author

Still can't seem to get that readme to work, not sure what the issue is?
Screenshot 2024-02-19 at 10 06 00

@brijeshb42
Copy link
Contributor

@rluzists1 It won't work right now since the image you added only lives in your branch. Once your changes are merged to master and the docs site gets the updated code, it'll have the new image and your changes in README will also reflect the image.

@rluzists1
Copy link
Contributor Author

Thank you @brijeshb42 that's really helpful, I'll stop stressing about it then!

In that case, all changes on my end are complete/look good; will wait for your review @danilo-leal before merging :)

Copy link
Contributor

@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.

Looks good! One question, though — we're maintaining the "Become a Diamond Sponsor" button on the documentation's TOC. Have we now expanded our diamond sponsor slots for more than three? That'd explain the button (given that, in the past, whenever we hit three, we used to remove it!).

@rluzists1
Copy link
Contributor Author

Thanks for reviewing @danilo-leal and yes, we have in theory! I've removed other language that referenced it on the site (1/3 slots), although I haven't made changes to the parts of the code that reference it (const maxNumberOfDimaondSponsors = 3 in DiamondSponsors.txt) just yet. Will tackle this if/when we want to add a 4th diamond sponsor.

All good for me to click squash and merge then?

Signed-off-by: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 19, 2024
@rluzists1 rluzists1 merged commit 1476da5 into mui:master Feb 19, 2024
22 checks passed
@rluzists1 rluzists1 deleted the marblism-diamond-sponsorship branch March 19, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
website Pages that are not documentation-related, marketing-focused.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants