chore: improve floating schedule button #23873
Conversation
WalkthroughThe pull request updates the HTML template in packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts. It removes the mr-3 class from the button icon container and adds the ml-3 class to the button text container, adjusting horizontal spacing between the icon and label. No function signatures, exports, logic, or behavior are changed; only presentational markup is modified. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
keithwillcode
added
core
dosubot
bot
added
the
🧹 Improvements
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts (1)
14-16: Add accessible name when rendering icon-only button.If
buttonTextis empty, this button has no accessible name (WCAG 4.1.2). Providearia-label(ideally from a prop) and fall back tobuttonText.-return `<button class="z-[999999999999] hidden fixed md:bottom-6 bottom-4 md:right-10 right-4 md:left-10 left-4 ${buttonClasses.join( +return `<button aria-label="${buttonText || 'Open scheduling'}" class="z-[999999999999] hidden fixed md:bottom-6 bottom-4 md:right-10 right-4 md:left-10 left-4 ${buttonClasses.join(If i18n is required, introduce a
buttonAriaLabel?: stringparam and prefer it over the fallback.
🧹 Nitpick comments (4)
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts (4)
34-34: Avoid phantom spacing when buttonText is empty.With
ml-3on an always-present flex item, you still get a 0‑width item that adds left margin. Either render the text node only when non-empty, or use container gaps.Option A — conditional margin only when text exists:
-<div id="button" class="ml-3 font-semibold leading-5 antialiased">${buttonText}</div> +<div id="button" class="font-semibold leading-5 antialiased ${buttonText ? 'ml-3' : ''}">${buttonText}</div>Option B — prefer Tailwind gap so spacing auto-collapses (works LTR/RTL):
- )} flex h-16 origin-center bg-red-50 transform cursor-pointer items-center + )} flex h-16 origin-center bg-red-50 transform cursor-pointer items-center gap-x-3-<div id="button" class="ml-3 font-semibold leading-5 antialiased">${buttonText}</div> +<div id="button" class="font-semibold leading-5 antialiased">${buttonText}</div>
19-19: Inline style parsing bug (missing semicolon).
z-indexis appended tocolorvalue and ignored by the browser.-style="background-color:${buttonColor}; color:${buttonTextColor} z-index: 10001"> +style="background-color:${buttonColor}; color:${buttonTextColor}; z-index: 10001">
21-31: SVG attributes are JSX-style; use kebab-case in plain HTML.
strokeLinecap|strokeLinejoin|strokeWidthwon’t apply in HTML; usestroke-linecap|stroke-linejoin|stroke-width. Also mark decorative SVG as hidden from AT.- <svg - class="h-7 w-7" - fill="none" - stroke="currentColor" - viewBox="0 0 24 24" - xmlns="http://www.w3.org/2000/svg"> - <path - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth="2" + <svg + class="h-7 w-7" + fill="none" + stroke="currentColor" + viewBox="0 0 24 24" + xmlns="http://www.w3.org/2000/svg" + aria-hidden="true" focusable="false"> + <path + stroke-linecap="round" + stroke-linejoin="round" + stroke-width="2"
1-11: Prefer named export over default.Improves tree‑shaking and refactors.
-const getHtml = ({ +export const getHtml = ({-export default getHtml; +// export { getHtml }; // if you keep default for now, consider switching in a follow-upThis is breaking for importers; gate behind a codemod or follow-up PR if widely used.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts (1)
20-20: Spacing move off the icon looks good.Removing margin from the icon container is the right direction.
| rounded-full py-4 px-6 text-base outline-none drop-shadow-md transition focus:outline-none fo | ||
| cus:ring-4 focus:ring-gray-600 focus:ring-opacity-50 active:scale-95" |
There was a problem hiding this comment.
Broken Tailwind class: focus:ring-4 split across lines.
Currently renders fo and cus:ring-4 as two invalid classes; focus ring won’t apply.
-rounded-full py-4 px-6 text-base outline-none drop-shadow-md transition focus:outline-none fo
-cus:ring-4 focus:ring-gray-600 focus:ring-opacity-50 active:scale-95"
+rounded-full py-4 px-6 text-base outline-none drop-shadow-md transition focus:outline-none focus:ring-4 focus:ring-gray-600 focus:ring-opacity-50 active:scale-95"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rounded-full py-4 px-6 text-base outline-none drop-shadow-md transition focus:outline-none fo | |
| cus:ring-4 focus:ring-gray-600 focus:ring-opacity-50 active:scale-95" | |
| rounded-full py-4 px-6 text-base outline-none drop-shadow-md transition focus:outline-none focus:ring-4 focus:ring-gray-600 focus:ring-opacity-50 active:scale-95" |
🤖 Prompt for AI Agents
In packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts around
lines 17 to 18, a Tailwind class token was accidentally split across the line
into "fo" and "cus:ring-4", breaking the intended "focus:ring-4" class; restore
the full token by joining the split parts so the class reads exactly
"focus:ring-4" (and remove any stray partial tokens) so the focus ring styling
applies correctly.
| </svg> | ||
| </div> | ||
| <div id="button" class="font-semibold leading-5 antialiased">${buttonText}</div> | ||
| <div id="button" class="ml-3 font-semibold leading-5 antialiased">${buttonText}</div> |
There was a problem hiding this comment.
Escape buttonText to prevent HTML injection.
buttonText is interpolated into raw HTML. Escape HTML entities to avoid XSS in embed contexts.
-<div id="button" class="ml-3 font-semibold leading-5 antialiased">${buttonText}</div>
+<div id="button" class="ml-3 font-semibold leading-5 antialiased">${(buttonText ?? '').replace(/[&<>"']/g, (c) => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c]!) )}</div>If you prefer clarity, extract a small escapeHtml helper above and call it here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div id="button" class="ml-3 font-semibold leading-5 antialiased">${buttonText}</div> | |
| <div id="button" class="ml-3 font-semibold leading-5 antialiased">${(buttonText ?? '').replace(/[&<>"']/g, (c) => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c]!))}</div> |
🤖 Prompt for AI Agents
In packages/embeds/embed-core/src/FloatingButton/FloatingButtonHtml.ts around
line 34, the buttonText is directly interpolated into the HTML causing possible
XSS; create a small escapeHtml helper (or import a safe encoder) that replaces
&, <, >, ", ' and / with their corresponding HTML entities and then use
escapeHtml(buttonText) when building the div string so the rendered HTML is
safe.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
this allows us to have a button without text without breaking the UI
before
after
