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 phone and email link block #2142

Merged
merged 14 commits into from
Jun 25, 2024
Merged

Add phone and email link block #2142

merged 14 commits into from
Jun 25, 2024

Conversation

SebiVPS
Copy link
Contributor

@SebiVPS SebiVPS commented Jun 5, 2024


PR Checklist

  • Verify if the change requires a changeset. See CONTRIBUTING.md
  • Provide screenshots/screencasts if the change contains visual changes
Screenshots/screencasts Screenshot 2024-06-05 at 08 20 08

@SebiVPS SebiVPS requested a review from johnnyomair June 5, 2024 06:53
@nsams
Copy link
Member

nsams commented Jun 5, 2024

I remember we discussed if we need this or not, but that is years ago. We decided No back then.

@SebiVPS
Copy link
Contributor Author

SebiVPS commented Jun 5, 2024

@nsams: @johnnyomair and @thomasdax98 decided that we should add these two blocks to the library. Meanwhile there are multiple implementations in many different repos around. So instead of add another copy to the comet-starter, we add it to the library and remove the other copies. The components itself are pretty common and can be used for every use case when they are needed.

@johnnyomair
Copy link
Collaborator

@nsams the problem with supporting mailto: and tel: in ExternalLinkBlock only is that editors don't know about them. We thought about adding a fancy UI/UX feature to fix this (e.g., detecting emails automatically and add the mailto: prefix), but couldn't come up with an elegant solution. Furthermore, editors would still need to know that they have to select "External link" in the link block for a phone number, which isn't intuitive. So we decided having two more options is the way to go.

@johnnyomair
Copy link
Collaborator

@SebiVPS please rebase and resolve conflicts.

@SebiVPS SebiVPS force-pushed the links-email-phone branch from 0c58569 to dd8545d Compare June 5, 2024 07:40
@SebiVPS SebiVPS requested a review from johnnyomair June 11, 2024 10:22
@johnnyomair
Copy link
Collaborator

Why are the blocks in @comet/blocks-api but in @comet/cms-admin, and not @comet/blocks-admin?

@SebiVPS
Copy link
Contributor Author

SebiVPS commented Jun 19, 2024

Why are the blocks in @comet/blocks-api but in @comet/cms-admin, and not @comet/blocks-admin?

@johnnyomair i put it there because there are more other blocks around in @comet/cms-admin. In @comet/blocks-admin there is only a deprecated SpaceBlockand a YoutTubeBlock.
We can move it to the other package if you want. I don't know where it really belongs.

@johnnyomair
Copy link
Collaborator

Why are the blocks in @comet/blocks-api but in @comet/cms-admin, and not @comet/blocks-admin?

@johnnyomair i put it there because there are more other blocks around in @comet/cms-admin. In @comet/blocks-admin there is only a deprecated SpaceBlockand a YoutTubeBlock. We can move it to the other package if you want. I don't know where it really belongs.

I'd say move it @comet/cms-api. We plan to dissolve @comet/blocks-api and @comet/blocks-admin into the respective cms packages.

@SebiVPS SebiVPS requested a review from johnnyomair June 25, 2024 06:06
@johnnyomair johnnyomair merged commit 73dfb61 into main Jun 25, 2024
11 checks passed
@johnnyomair johnnyomair deleted the links-email-phone branch June 25, 2024 07:22
fichtnerma pushed a commit that referenced this pull request Jul 18, 2024
---------

Co-authored-by: Johannes Obermair <48853629+johnnyomair@users.noreply.github.com>
Co-authored-by: Johannes Obermair <johannes.obermair@vivid-planet.com>
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.

3 participants