Conversation
Rohit0301
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (22 flaky)✅ 3952 passed · ❌ 0 failed · 🟡 22 flaky · ⏭️ 86 skipped
🟡 22 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
aniketkatkar97
left a comment
aniketkatkar97
left a comment
There was a problem hiding this comment.
Also test coverage is missing
| <span | ||
| className="code-copy-message" | ||
| data-copied={copied} | ||
| data-testid="copied-message"> | ||
| {t('label.copied')} | ||
| </span> | ||
| <button | ||
| className="code-copy-button" | ||
| data-copied={copied} | ||
| data-testid="code-block-copy-icon" | ||
| type="button" | ||
| onClick={handleCopy}> | ||
| <img alt="copy" height={24} src={CopyIcon} width={24} /> | ||
| </button> |
There was a problem hiding this comment.
Why are we not using untitled ui components here?
There was a problem hiding this comment.
This will also go in the minor release, let me use the antd instead button
Code Review ✅ Approved 6 resolved / 6 findingsServiceDoc panel now supports admonitions and code copy functionality. Resolved multiple stability issues, including redundant set definitions, missing reference forwarding, and memory leaks from uncleared timeouts. ✅ 6 resolved✅ Bug: Admonition content is double-converted through markdown-to-HTML
✅ Bug: contentRef will always be null — NodeViewContent doesn't forward refs
✅ Edge Case: setTimeout in handleCopy not cleared on unmount
✅ Quality: ADMONITION_TYPES set is redundant — regex already constrains values
✅ Security: AdmonitionNode type attribute is not validated on parse
...and 1 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
| * where the ServiceDocPanel is visible with code blocks and sections. | ||
| */ | ||
| const goToMysqlConnectionStep = async (page: Page, serviceName: string) => { | ||
| await page.goto('/databaseServices/add-service'); |
There was a problem hiding this comment.
Please add waituntil domcontentloaded here and all other places where we are redirecting
|
|
||
| // Verify state resets after 2s timer | ||
| await expect(copyButtonWrapper).toHaveAttribute('data-copied', 'false', { | ||
| timeout: 3000, |
There was a problem hiding this comment.
Why are we adding a timeout here? Please remove that wherever we have added it.
2 participants
Describe your changes:
collate PR - https://github.com/open-metadata/openmetadata-collate/pull/3890
Added admonition (note/warning/tip etc.) rendering and code block copy button support to the Service Documentation panel.
Fixes #27705
I worked on ... because ...
https://github.com/user-attachments/assets/7346feee-5cde-4eb3-a4dc-9831a9a6a9f6
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
ServiceDocPanelto verify content rendering, section highlighting, and copy functionality.CodeBlockComponentto use Ant DesignButtonandTooltipfor the copy action.service-doc-panel.lessfor improved copy button positioning and styling.This will update automatically on new commits.