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

[survey] Remove survey promotion items #30122

Merged
merged 10 commits into from
Dec 14, 2021

Conversation

danilo-leal
Copy link
Contributor

@danilo-leal danilo-leal commented Dec 8, 2021

To be merged before this week's release as the survey closes for answers on Tuesday, Dec 14th.

  • Remove banner from the marketing pages.
  • Remove the card from docs' table of contents.
  • Remove notification.
  • Make the banner component generic so it can be used in the future again.

@danilo-leal danilo-leal added docs Improvements or additions to the documentation website Pages that are not documentation-related, marketing-focused. labels Dec 8, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 8, 2021

No bundle size changes

Generated by 🚫 dangerJS against 1d5808f

@siriwatknp siriwatknp added the on hold There is a blocker, we need to wait label Dec 9, 2021
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

👍

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 9, 2021

Should we remove the notification too?

@danilo-leal danilo-leal changed the title [survey] Remove banner and card [survey] Remove survey promotion items Dec 9, 2021
@mbrookes
Copy link
Member

mbrookes commented Dec 9, 2021

I'd suggest reverting the previous commits rather than manually removing the changes.

@danilo-leal
Copy link
Contributor Author

What's the difference between this approach and reverting the commits? I really don't know.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 10, 2021

One point I wonder about: Should we leave some infrastructure in place for the next time we want to add a banner? For instance, we have kept SurveyBanner.js from #29950.


What's the difference between this approach and reverting the commits? I really don't know.

I think that it's well covered in https://www.atlassian.com/git/tutorials/undoing-changes/git-revert.

@mbrookes
Copy link
Member

mbrookes commented Dec 11, 2021

What's the difference between this approach and reverting the commits?

I just meant for the future. It's quicker and easier, and less likely to miss something. Also easier to review, as you're only concerned with checking that the appropriate commits are reverted.

@mbrookes
Copy link
Member

mbrookes commented Dec 11, 2021

To be merged only on Dec 13th

Or at any point. The docs shouldn't published at a newer commit in the meantime, as that risks the content being out of sync with the published packages.

@danilo-leal danilo-leal requested a review from mnajdova December 13, 2021 15:41
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

If we want to keep the banner generic, we should still import it from all the pages we had before, just use the new name WebsiteBanner

docs/src/components/home/WebsiteBanner.tsx Outdated Show resolved Hide resolved
@danilo-leal danilo-leal requested a review from mnajdova December 13, 2021 15:55
@danilo-leal danilo-leal removed the on hold There is a blocker, we need to wait label Dec 13, 2021
@danilo-leal danilo-leal self-assigned this Dec 14, 2021
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.

Updated to use feature toggle to hide the banner.

@siriwatknp siriwatknp merged commit 95eac12 into mui:master Dec 14, 2021
@danilo-leal danilo-leal deleted the remove-survey-components branch December 14, 2021 11:42
@oliviertassinari
Copy link
Member

Nice new AppHeaderBanner component 👌

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 23, 2021

Note: this PR is not enough to remove all the survey, it's still present in MUI X, e.g.

Screenshot 2021-12-23 at 11 33 37

https://mui.com/components/data-grid/editing/

It's a bit confusing to see the banner, clicking on it to have:

Screenshot 2021-12-23 at 11 36 10

cc @mui-org/x

@danilo-leal
Copy link
Contributor Author

danilo-leal commented Dec 23, 2021

The survey card isn't the only confusing thing though, https://mui.com/components/data-grid/ is still showing to me as v5.2.3, click in any Core component and then v5.2.5. So maybe it's something even bigger than the scope of this PR?!

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 23, 2021

Yes, definitely. It suggests that we need to upgrade the dependency on the mono-repo in X, it hasn't been updated for > 3 weeks. I wonder if renovate could do it for us every week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation website Pages that are not documentation-related, marketing-focused.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants