-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(tabs): After the global style is changed, an error is reported in the UI screenshot of tabs. #80
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce a new test case in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/tabs/xdesign.spec.ts (1)
55-71
: Good test structure, consider these enhancementsThe new test case for different tab positions is well-structured and covers important scenarios. Here are some suggestions to improve its robustness:
- Consider adding a test for the 'top' position to ensure all possible positions are covered.
- Add explicit waits or assertions after changing positions to ensure the UI has updated before taking screenshots. This can prevent flaky tests.
- Verify that the tab position has actually changed before taking screenshots. This could be done by checking a specific CSS property or class that indicates the position.
Here's a suggested enhancement to the test case:
test('各个位置--UI截图', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tabs#position') const demo = page.locator('#position .pc-demo .tiny-tabs') await expect(demo).toBeInViewport() // Helper function to change position and verify async function changePositionAndCapture(position: string) { await page.locator('label').filter({ hasText: `${position} 显示` }).click() await expect(demo).toHaveAttribute('data-position', position) await expect(demo).toBeInViewport() await expect(demo).toHaveScreenshot(`${position}.png`) } // Test all positions await expect(demo).toHaveAttribute('data-position', 'left') await expect(demo).toHaveScreenshot('left.png') await changePositionAndCapture('top') await changePositionAndCapture('bottom') await changePositionAndCapture('right') }This enhancement adds a test for the 'top' position, includes a wait for the position to change (assuming there's a
data-position
attribute), and uses a helper function to reduce code duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (18)
tests/tabs/xdesign.spec.ts-snapshots/basic-usage-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/basic-usage-click-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/bottom-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/left-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/more-show-all-btn-hover-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/more-show-all-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/more-show-all-click-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/right-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/size-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/size-hover-0-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/size-hover-1-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/size-hover-2-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-second-layer-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-second-layer-currency-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-second-layer-hover-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-second-layer-selected-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-separator-chromium-win32.png
is excluded by!**/*.png
tests/tabs/xdesign.spec.ts-snapshots/tabs-separator-hover-chromium-win32.png
is excluded by!**/*.png
📒 Files selected for processing (1)
- tests/tabs/xdesign.spec.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
tests/tabs/xdesign.spec.ts (1)
55-71
: Overall, good addition to the test suiteThis new test case for different tab positions is a valuable addition to the test suite. It aligns well with the PR objectives of addressing UI issues related to tabs. The test covers multiple positions and includes user interactions, which will help catch potential regressions in the tab positioning functionality.
While the test is generally well-structured, implementing the suggested enhancements would further improve its reliability and coverage. Great job on expanding the test coverage!
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit