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

Add support for custom labels in createOneOfBlock #2413

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

fichtnerma
Copy link
Contributor

@fichtnerma fichtnerma commented Aug 12, 2024


Add support for Icons and other ReactNodes for OneOfBlock Tab Labels

PR Checklist

  • Verify if the change requires a changeset. See CONTRIBUTING.md
  • Link to the respective task if one exists: COM-862
  • Provide screenshots/screencasts if the change contains visual changes
Screenshots/screencasts

image

Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

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

Please add a usage in Demo, either video block or media block

.changeset/fluffy-points-think.md Outdated Show resolved Hide resolved
@SebiVPS
Copy link
Contributor

SebiVPS commented Aug 16, 2024

I think we need some kind of special solution to avoid tinting the icons. As you can notice in the screenshot the youtube icon (as well as the vimeo icon) should not be tinted with the selected (or not selected) tint color.

@fichtnerma
Copy link
Contributor Author

I think we need some kind of special solution to avoid tinting the icons. As you can notice in the screenshot the youtube icon (as well as the vimeo icon) should not be tinted with the selected (or not selected) tint color.

The Tinting of the Icons appears to be a Comet problem and also appears on the Storybook Icons Page

@johnnyomair
Copy link
Collaborator

I think we need some kind of special solution to avoid tinting the icons. As you can notice in the screenshot the youtube icon (as well as the vimeo icon) should not be tinted with the selected (or not selected) tint color.

@thomasdax98 maybe it's not a good idea to have colored versions of the brand icons after all. MUI also doesn't do this: https://mui.com/material-ui/material-icons/?query=youtu&selected=YouTube

@johnnyomair
Copy link
Collaborator

This PR has now 636 changed files and conflicts. Please fix, thanks!

@fichtnerma fichtnerma force-pushed the COM-862-support-icons-in-one-of-block-tabs branch from 0c38888 to 9461c4e Compare October 22, 2024 20:26
@johnnyomair
Copy link
Collaborator

I think we need some kind of special solution to avoid tinting the icons. As you can notice in the screenshot the youtube icon (as well as the vimeo icon) should not be tinted with the selected (or not selected) tint color.

@thomasdax98 maybe it's not a good idea to have colored versions of the brand icons after all. MUI also doesn't do this: https://mui.com/material-ui/material-icons/?query=youtu&selected=YouTube

We discussed this today with UX and decided to remove the tinting of the icons. @fichtnerma please remove them, thanks!

packages/admin/admin-icons/.DS_Store Outdated Show resolved Hide resolved
packages/admin/admin-icons/icons/youtube.svg Outdated Show resolved Hide resolved
demo/admin/src/common/blocks/MediaBlock.tsx Outdated Show resolved Hide resolved
packages/admin/admin-icons/icons/youtube.svg Outdated Show resolved Hide resolved
packages/admin/admin-icons/icons/vimeo.svg Outdated Show resolved Hide resolved
johnnyomair
johnnyomair previously approved these changes Jan 23, 2025
@johnnyomair johnnyomair changed the title Support Icons in OneOfBlock Tabs Add support for custom labels in createOneOfBlock Jan 23, 2025
@johnnyomair johnnyomair force-pushed the COM-862-support-icons-in-one-of-block-tabs branch from b574f9f to 1d0d6ac Compare January 23, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants