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 embed option to share modal #92

Merged
merged 14 commits into from
Dec 6, 2024
Merged

Conversation

farisd16
Copy link
Member

@farisd16 farisd16 commented Sep 14, 2024

This PR adds the embed option to the share modal.

New option in the share modal

Upon clicking on embed, a text of two lines is copied to the clipboard. If the text is pasted in a pull request / issue, an image of the diagram together with a link to editing a copy can be seen.

This is a screenshot of an example embedding in #98 coming from a test server of Apollon:
Example PR with embedding

In the future, when version management of diagrams is implemented, as described in #97, the embedding will display the newest version of a diagram.

@farisd16 farisd16 added the enhancement New feature or request label Sep 14, 2024
@farisd16 farisd16 self-assigned this Sep 14, 2024
@FelixTJDietrich
Copy link
Contributor

Hmm, basically everyone uses GitHub using dark mode, we should add a background 🤔

Also maybe you can switch around the buttons:
Edit, Embed, Collaborate
Give Feedback, See Feedback

This might be better what do you think?

@FelixTJDietrich
Copy link
Contributor

Why are the recently shared diagrams separate? Do you copy each part? I thought we add an additional "Recently shared embed" if it exists and have one button that copies the markdown text for the whole embed

@farisd16 farisd16 linked an issue Sep 26, 2024 that may be closed by this pull request
Copy link
Contributor

@egenerse egenerse left a comment

Choose a reason for hiding this comment

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

I tested and it works, I see the screen shot of the diagram in the github issue after pasting it

@FelixTJDietrich
Copy link
Contributor

IMO this should read Copy Embed for embeddings and Copy Link for just the shared links of edit, collaborate, see feedback, and give feedback
image

Copy link
Contributor

@FelixTJDietrich FelixTJDietrich left a comment

Choose a reason for hiding this comment

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

Worked as expected when testing on the consolidated branch, code looks also good to me

@FelixTJDietrich FelixTJDietrich merged commit 18f41a5 into main Dec 6, 2024
3 checks passed
@FelixTJDietrich FelixTJDietrich deleted the feature/embed-diagram branch December 6, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedding test
3 participants