Skip to content

Conversation

@alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Feb 19, 2024

Fix #41129 (comment)

  • For carbon ads, just looking ad the href attibutes. if it contains cabonads I remove the arrow
  • Form images, I only found two places in docs/data/**/*.md that contains _blank So I added a class by hand to them

@alexfauquette alexfauquette added the scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). label Feb 19, 2024
@mui-bot
Copy link

mui-bot commented Feb 19, 2024

// Allows to remove link arrows for images
display: 'none',
},
'& a[href*="carbonads"][target="_blank"]::after': {
Copy link
Member

Choose a reason for hiding this comment

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

It can be more than carbon ads, I think we need a more generic selectors.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about targeting links inside those 3 selectors:

document.querySelector('.ea-placement')
document.querySelector('#carbonads')
document.querySelector('.carbonads')

https://github.com/mui/material-ui/blob/master/docs/src/modules/components/Ad.js/#L157-L160

Copy link
Member

@oliviertassinari oliviertassinari Feb 20, 2024

Choose a reason for hiding this comment

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

Instead, Could we create/add a new class name to an existing DOM node around the ad?

It would make it less brittle to future changes. For example, what if we implement: https://www.notion.so/mui-org/docs-infra-Ad-networks-fcd294a093354a94a329ab9852433fdc?pvs=4#ee1fd00fd3b642f28439c7d1f211695f?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you know what is .ea-placement for? The search in codebase returns 0 files with this text 🤔

Could it be an old elements that does not exist anymmore?

Copy link
Member

@oliviertassinari oliviertassinari Feb 21, 2024

Choose a reason for hiding this comment

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

Do you know what is .ea-placement for?

ea was for EthicalAds

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. type: regression A bug, but worse, it used to behave as expected. labels Feb 20, 2024
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.

👍 Nice catch!

@alexfauquette alexfauquette merged commit 12948d8 into mui:master Feb 21, 2024
oliviertassinari added a commit that referenced this pull request Feb 21, 2024
Style spread on two different files will be hard to keep in sync.
Follow-up on #41181
Also simplify the CSS selector for performance.
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 8, 2024
Style spread on two different files will be hard to keep in sync.
Follow-up on mui#41181
Also simplify the CSS selector for performance.
@oliviertassinari oliviertassinari removed the type: bug It doesn't behave as expected. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants