-
Notifications
You must be signed in to change notification settings - Fork 30
Add new figma embed script host embed.figma.com #205
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for the new Figma embed script host embed.figma.com
alongside the existing www.figma.com
host. This change aligns with Figma's updated embedding documentation.
Key changes:
- Updated Figma embed configuration to support multiple script hosts
- Refactored test suite to validate both old and new embed URLs
- Modified iframe filtering to handle the expanded host list
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
lib/qiita/markdown/embed/figma.rb | Changed from single SCRIPT_HOST to SCRIPT_HOSTS array containing both www.figma.com and embed.figma.com |
lib/qiita/markdown/transformers/filter_iframe.rb | Updated reference from SCRIPT_HOST to SCRIPT_HOSTS to accommodate the new array format |
spec/qiita/markdown/processor_spec.rb | Refactored tests to iterate over both embed URLs, ensuring comprehensive coverage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @acronhub, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request expands the supported domains for Figma embeds by including embed.figma.com
. This change ensures that content embedded from this new Figma host will be correctly processed and displayed, improving compatibility and user experience for Figma content.
Highlights
- New Figma Embed Host Added: The new Figma embed script host, embed.figma.com, has been added to the list of allowed domains, enabling support for Figma content embedded from this new source.
- Figma Embed Host Configuration Update: The SCRIPT_HOST constant for Figma embeds has been refactored into a SCRIPT_HOSTS array, allowing for the inclusion of multiple valid Figma embed domains.
- Figma Embed Test Refactoring: The test suite for Figma embed code has been updated to iterate over a list of script URLs, ensuring comprehensive coverage for both existing and newly supported Figma embed hosts.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly adds the new Figma embed host embed.figma.com
to the whitelist and updates the corresponding tests. The changes are logical and well-implemented. I have one suggestion to improve the maintainability of the tests by dynamically generating test cases from the application's constants, which will make the test suite more robust to future changes.
[ | ||
"www.figma.com/embed?embed_host=share&url=https%3A%2F%2Fwww.figma.com", | ||
"embed.figma.com/design/nrPSsILSYjesyc5UHjYYa4?embed_host=share", | ||
].each do |script_url| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test refactoring is a good step. To make it even more robust and maintainable, you could generate the test cases from the Qiita::Markdown::Embed::Figma::SCRIPT_HOSTS
constant. This would prevent duplicating the hostnames in the tests and ensure that all whitelisted hosts are automatically included in the test suite when the constant is updated.
test_cases = {
"www.figma.com" => "www.figma.com/embed?embed_host=share&url=https%3A%2F%2Fwww.figma.com",
"embed.figma.com" => "embed.figma.com/design/nrPSsILSYjesyc5UHjYYa4?embed_host=share",
}
Qiita::Markdown::Embed::Figma::SCRIPT_HOSTS.map { |host| test_cases[host] }.compact.each do |script_url|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
What