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

feat(modal): [modal] add e2e-test and update modal demo of problem #2567

Merged
merged 3 commits into from
Nov 28, 2024

Conversation

James-9696
Copy link
Collaborator

@James-9696 James-9696 commented Nov 27, 2024

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced modal functionality with new properties and methods for better usability, including the ability to close modals using the ESC key.
    • Introduced a revert method to restore modal size and position after maximizing.
  • Bug Fixes

    • Improved error handling in modal event management to prevent closure under certain conditions.
  • Tests

    • Added multiple test cases to ensure the functionality of modals, including resizing, status indicators, and event handling.
    • Verified the behavior of modals with customizable features and interactions.
  • Refactor

    • Streamlined component imports and usage across the modal components for consistency and clarity.

Copy link

coderabbitai bot commented Nov 27, 2024

Walkthrough

This pull request encompasses extensive modifications to various Vue components related to modal functionality. Key changes include renaming imported modal components from TinyModal to Modal, updating method calls accordingly, and introducing new test cases to validate these functionalities. Additionally, several components have been streamlined for clarity, and some have been removed entirely. The changes aim to enhance the consistency and usability of modal interactions across the application.

Changes

File Change Summary
examples/sites/demos/pc/app/modal/basic-usage-composition-api.vue Updated import statements and component references from TinyButton, TinyModal, TinyNotify to Button as TinyButton, Modal, and Notify.
examples/sites/demos/pc/app/modal/basic-usage.vue Renamed imported components and updated method calls from TinyModal and TinyNotify to Modal and Notify.
examples/sites/demos/pc/app/modal/basic-usage.spec.ts Added a new test suite for basic modal functionality using Playwright.
examples/sites/demos/pc/app/modal/footer-slot-composition-api.vue Deleted file containing a modal component with a customizable footer.
examples/sites/demos/pc/app/modal/footer-slot.vue Deleted file with a modal component implementing a customizable footer.
examples/sites/demos/pc/app/modal/message-closable-composition-api.vue Deleted file for a closable message modal component.
examples/sites/demos/pc/app/modal/message-closable.vue Deleted file for a closable message modal component.
examples/sites/demos/pc/app/modal/message-close-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-close.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-close.spec.ts Added a new test case for modal message closure functionality.
examples/sites/demos/pc/app/modal/message-event-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-event.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-event.spec.ts Added a new test case for message events in the modal.
examples/sites/demos/pc/app/modal/message-id-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-id.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-id.spec.ts Added a new test case to prevent duplicate message prompts.
examples/sites/demos/pc/app/modal/message-top-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/message-top.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-event-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-event.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-event.spec.ts Added a new test case for modal events.
examples/sites/demos/pc/app/modal/modal-fn-slots-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-fn-slots.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-footer-composition-api.vue Updated import statements and added a new property to the modal component.
examples/sites/demos/pc/app/modal/modal-footer.vue Updated import statements and added a new property to the modal component.
examples/sites/demos/pc/app/modal/modal-header-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-header.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-lock-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-lock.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-mask-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-mask.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-other-composition-api.vue Updated import statements and added a new property to the modal component.
examples/sites/demos/pc/app/modal/modal-other.vue Updated import statements and added a new property to the modal component.
examples/sites/demos/pc/app/modal/modal-resize-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-resize.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-size-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/modal-size.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/status-composition-api.vue Updated import statements and method calls from TinyModal to Modal.
examples/sites/demos/pc/app/modal/status.vue Updated import statements and method calls from TinyModal to Modal.
packages/renderless/src/modal/index.ts Added a revert method and updated existing methods to enhance modal functionality.

Possibly related PRs

Suggested labels

enhancement, bug

Suggested reviewers

  • zzcr

🐰 In the meadow, the modals bloom,
With buttons bright, dispelling gloom.
A click, a close, all flows so fine,
With names so clear, they brightly shine.
Hooray for changes, let joy unfold,
In our little app, new tales are told! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request label Nov 27, 2024
@James-9696 James-9696 added the bug Something isn't working label Nov 27, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 44

🧹 Outside diff range and nitpick comments (72)
examples/sites/demos/pc/app/modal/message-id.spec.ts (1)

1-9: Consider enhancing the e2e test suite structure.

As this is part of adding e2e tests for the modal component, consider these architectural improvements:

  1. Add a test configuration file to handle common setup (like page error handling)
  2. Create test utilities for common modal operations
  3. Consider adding visual regression tests for modal animations
  4. Add accessibility tests to verify modal behavior with screen readers

Would you like assistance in implementing any of these architectural improvements?

examples/sites/demos/pc/app/modal/message-close.spec.ts (1)

3-4: Consider improving error handling and test naming

  1. Consider using a more descriptive English test name for better international collaboration.
  2. The error handler could be more specific about the expected errors.

Consider this improvement:

-test('消息的关闭和延时', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+test('should auto-close message after delay', async ({ page }) => {
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })
examples/sites/demos/pc/app/modal/message-top.spec.ts (1)

6-6: Consider using a more reliable selector strategy

The class selector .type__message might be fragile if the CSS classes change. Consider adding a dedicated test attribute.

Add a test attribute to the modal component:

- <div class="type__message">
+ <div class="type__message" data-testid="modal-message">
examples/sites/demos/pc/app/modal/message-close-composition-api.vue (2)

Line range hint 13-18: Consider extracting duration constant and internationalizing message

The modal implementation is correct, but consider these improvements:

  1. Extract the duration as a constant: const AUTO_CLOSE_DURATION = 5000
  2. Consider using i18n for the message text to support internationalization
+const AUTO_CLOSE_DURATION = 5000
+
 function btnClick() {
   Modal.message({
     status: 'info',
-    message: '自定义消息的内容可关闭,5s 后得自动关闭',
+    message: t('modal.message.autoClose'),
     messageClosable: true,
-    duration: 5000
+    duration: AUTO_CLOSE_DURATION
   })
 }

Test coverage for modal message functionality needs improvement

The current test coverage for the modal message functionality is insufficient:

  • The existing test only verifies the presence of text but doesn't validate the messageClosable functionality
  • The auto-close behavior after 5s is not properly tested
  • No test cases for actually closing the message via the close button

Suggested test improvements:

  • Add test case to verify the close button is visible when messageClosable is true
  • Add test case to verify clicking the close button dismisses the message
  • Add test case to verify the message auto-closes after the specified duration
🔗 Analysis chain

Line range hint 13-18: Verify e2e test coverage for modal message functionality

Since this PR aims to add e2e tests, let's verify the test coverage for this modal message functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for e2e tests related to modal message functionality
rg -t ts -t js "Modal\.message.*messageClosable.*duration" "test|tests|e2e|cypress|__tests__"

# Search for test files specifically related to this component
fd -e spec.ts -e spec.js -e test.ts -e test.js "message-close" "test" "tests" "e2e" "cypress" "__tests__"

Length of output: 568


Script:

#!/bin/bash
# Search for test files in the entire repository
fd -e spec.ts -e spec.js -e test.ts -e test.js

# Search for modal message tests in any file
rg -t ts -t js "Modal\.message.*messageClosable.*duration"

# Search for any modal message related tests
rg -t ts -t js "Modal\.message.*test"

# Look for cypress test files that might contain modal tests
fd -e cy.ts -e cy.js

Length of output: 75566


Script:

#!/bin/bash
# Search for test files specifically in the modal directory
fd -e spec.ts -e spec.js -e test.ts -e test.js "message" "examples/sites/demos/pc/app/modal"

# Search for any modal message test content
rg -t ts -t js "Modal\.message" "examples/sites/demos/pc/app/modal"

# Look for test cases related to messageClosable
rg -t ts -t js "messageClosable" "examples/sites/demos/pc/app/modal"

Length of output: 3194


Script:

#!/bin/bash
# Check the content of message-close.spec.ts to see if it covers messageClosable functionality
cat "examples/sites/demos/pc/app/modal/message-close.spec.ts"

# Check the content of message-event.spec.ts for any message close event testing
cat "examples/sites/demos/pc/app/modal/message-event.spec.ts"

Length of output: 907

examples/sites/demos/pc/app/modal/message-event.spec.ts (2)

3-3: Consider using English test descriptions

For better maintainability and international collaboration, consider using English for test descriptions. This aligns with common open-source practices.

-test('消息的事件', async ({ page }) => {
+test('modal message events', async ({ page }) => {

3-10: Add test cleanup and improve structure

The test should clean up after itself to ensure a clean state for subsequent tests.

 test('modal message events', async ({ page }) => {
+  // Setup
   page.on('pageerror', (exception) => expect(exception).toBeNull())
   await page.goto('modal#message-event')
 
+  // Test
   const content = page.locator('.is-message')
   await page.getByRole('button', { name: '打开带事件消息' }).first().click()
   await expect(content.first()).toHaveText(/自定义消息的内容/)
   await expect(content.nth(1)).toHaveText(/show 事件触发了/)
+
+  // Cleanup - Close any open modals
+  await page.keyboard.press('Escape');
+  
+  // Verify cleanup
+  await expect(content.first()).not.toBeVisible();
 })
examples/sites/demos/pc/app/modal/modal-other.spec.ts (1)

4-4: Enhance error handling specificity

The current error handling catches all page errors without specific expectations. Consider handling specific error scenarios.

-page.on('pageerror', (exception) => expect(exception).toBeNull())
+page.on('pageerror', (exception) => {
+  console.error('Page error:', exception);
+  expect(exception.message).not.toContain('Modal');
+  expect(exception).toBeNull();
+});
examples/sites/demos/pc/app/modal/modal-header.spec.ts (2)

3-5: Consider adding test timeout and improving error handling

While the error handling is good, there are some improvements that could make the test more robust:

  1. Consider adding a test timeout for long-running operations
  2. The error handler could be more specific about the type of errors it catches
-test('自定义弹窗标题', async ({ page }) => {
+test('自定义弹窗标题', async ({ page }) => {
+  test.setTimeout(30000) // Add timeout for long running operations
+
   page.on('pageerror', (exception) => {
-    expect(exception).toBeNull()
+    console.error('Page error:', exception)
+    throw new Error(`Unexpected page error: ${exception}`)
   })

6-7: Consider using more specific selectors

The .tiny-modal__title selector might be fragile if there are multiple modals on the page.

Consider using a more specific selector or data-testid:

-const header = page.locator('.tiny-modal__title')
+const header = page.locator('[data-testid="modal-header-title"]')
examples/sites/demos/pc/app/modal/message-id.vue (2)

Line range hint 4-4: Document the reset-time prop and consider i18n

Two suggestions for improvement:

  1. Add a comment explaining the purpose of :reset-time="0"
  2. Consider using i18n for the button text "重复点击不出现多个" to support internationalization

Line range hint 18-22: Document the modal configuration and consider i18n

The implementation correctly uses a unique ID to prevent duplicate messages. Consider these improvements:

  1. Add JSDoc to document the purpose of the unique ID
  2. Use i18n for the message text
  3. Document the available status options ('info', etc.)
    btnClick() {
+     // Prevent duplicate messages by using a unique ID
      Modal.message({
        status: 'info', // Available options: 'info', 'success', 'error', 'warning'
-       message: '自定义消息具有唯一 id,所以不会重复出现',
+       message: this.$t('modal.uniqueMessage'),
        id: 'unique'
      })
    }
examples/sites/demos/pc/app/modal/message-close.vue (1)

Line range hint 18-23: Consider internationalizing the message text

The modal message contains hardcoded Chinese text. Consider using an internationalization system to support multiple languages.

Example approach using i18n:

  Modal.message({
    status: 'info',
-    message: '自定义消息的内容可关闭,5s 后得自动关闭',
+    message: t('modal.message.autoClose'),
    messageClosable: true,
    duration: 5000
  })
examples/sites/demos/pc/app/modal/modal-footer.spec.ts (1)

3-4: Consider using English test names for better international collaboration

While the current test name works, using English names would improve readability for international teams. Also, consider enhancing the error handling to be more specific about expected errors.

Consider this refactoring:

-test('自定义弹窗底部', async ({ page }) => {
+test('custom modal footer behavior', async ({ page }) => {
   page.on('pageerror', (exception) => expect(exception).toBeNull())

Additionally, consider adding more specific error handling:

  page.on('pageerror', (exception) => {
    console.error('Page error:', exception);
    throw exception;
  });
examples/sites/demos/pc/app/modal/modal-mask.spec.ts (2)

3-4: Consider enhancing error handling and test title.

  1. The error handling could be more specific about the types of errors you're expecting to catch.
  2. Consider using English for test titles to maintain consistency with code.

Consider this improvement:

-test('弹窗的遮罩', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+test('modal mask functionality', async ({ page }) => {
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })

1-14: Consider implementing a Page Object Model pattern.

To improve test maintainability and reusability, consider implementing a Page Object Model (POM) pattern for modal-related tests. This would:

  1. Centralize selectors and common interactions
  2. Make tests more readable and maintainable
  3. Reduce code duplication across modal tests
  4. Make it easier to update when the UI changes

Example structure:

// modalPage.ts
export class ModalPage {
  constructor(private page: Page) {}

  async openModal(buttonTestId: string) {
    await this.page.getByTestId(buttonTestId).click();
    // Add waiting for animation
  }

  async verifyModalMask(modalTestId: string) {
    const modal = this.page.getByTestId(modalTestId);
    await expect(modal).toHaveClass(/is__mask/);
  }

  // ... other common modal interactions
}

// modal-mask.spec.ts
const modalPage = new ModalPage(page);
await modalPage.openModal('modal-mask-button-1');
await modalPage.verifyModalMask('modal-1');
examples/sites/demos/pc/app/modal/message-event-composition-api.vue (2)

Line range hint 11-24: LGTM! Consider adding TypeScript type safety.

The Modal.message implementation is correct and maintains the expected event handling structure.

Consider adding TypeScript types for better maintainability:

-function btnClick() {
+interface ModalEvent {
+  show: (ev: Event) => void;
+  hide: (ev: Event) => void;
+  close: (ev: Event) => void;
+}
+
+function btnClick(): void {

28-30: LGTM! Consider enhancing error handling.

The event handling implementation is correct, with proper logging for debugging.

Consider adding error handling for the Modal.message call:

 function handleEvent(eventName, eventArgs) {
-  Modal.message({ status: 'info', message: `${eventName} 事件触发了` })
+  try {
+    Modal.message({ status: 'info', message: `${eventName} 事件触发了` })
+  } catch (error) {
+    console.error('Failed to show modal message:', error)
+  }
   console.log({ eventName, eventArgs })
 }
examples/sites/demos/pc/app/modal/modal-lock.spec.ts (2)

1-5: Consider enhancing error handling and test description.

While the basic setup is good, consider these improvements:

  1. Add a more specific error handler that logs the actual error message
  2. Consider using English for test descriptions to maintain consistency
-test('弹窗的锁定', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+test('modal locking functionality', async ({ page }) => {
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })

1-16: Consider architectural improvements for test maintainability.

To enhance the test suite's maintainability and reusability:

  1. Create a shared test fixture for modal-related tests
  2. Centralize selectors in a separate configuration file
  3. Consider implementing i18n for test descriptions and button labels

Example test fixture structure:

// fixtures/modal.ts
export const modalFixture = {
  getModal: (page) => page.locator('.tiny-modal'),
  getLayout: (page) => page.locator('#doc-layout'),
  buttons: {
    addScrollbar: '添加滚动条',
    restoreScrollbar: '还原滚动条',
    lockScrollbar: '仅锁定滚动条',
    confirm: '确定'
  }
};
examples/sites/demos/pc/app/modal/modal-size.spec.ts (2)

3-4: Consider using English test names for better compatibility.

While the Chinese test name '弹窗大小全屏' (Modal Size Fullscreen) is descriptive, using English names ensures better compatibility across different environments and tools.

Consider:

-test('弹窗大小全屏', async ({ page }) => {
+test('modal size fullscreen', async ({ page }) => {

Also, consider making the error handling more specific to catch only relevant modal-related errors rather than asserting all errors to be null.


5-7: Consider using more robust selectors.

The current selectors rely on CSS classes which could be fragile if the styling changes. Consider using data-testid attributes for more reliable test selectors.

Example improvement:

-  const modal = page.locator('.tiny-modal')
-  const box = page.locator('.type__confirm > .tiny-modal__box')
+  const modal = page.locator('[data-testid="modal"]')
+  const box = page.locator('[data-testid="modal-confirm-box"]')
examples/sites/demos/pc/app/modal/modal-fn-slots.spec.ts (2)

3-4: Consider enhancing error handling and test description.

While the error handling is good, consider these improvements:

  1. Add a more detailed test description in English as a comment
  2. Consider logging the actual error message for better debugging
+// Test: Verify modal slots functionality including default slot content and link rendering
 test('弹窗的插槽', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })

1-15: Consider adding more comprehensive test coverage.

The current test only covers basic slot functionality. Consider adding tests for:

  1. Error states
  2. Dynamic slot content
  3. Multiple slots
  4. Modal accessibility attributes

Would you like me to provide examples of additional test cases?

examples/sites/demos/pc/app/modal/modal-event.spec.ts (2)

3-4: Consider enhancing error handling and i18n support.

Two suggestions for improvement:

  1. The error handler could be more specific about what types of errors to expect/ignore
  2. Consider using i18n keys for test titles to support internationalization
-test('弹窗的事件', async ({ page }) => {
+test('modal.events.test', async ({ page }) => {
   page.on('pageerror', (exception) => expect(exception).toBeNull())

5-6: Consider using more robust selectors.

The .is-message class selector might be fragile if the CSS classes change. Consider using data-testid attributes for more stable test selectors.

-const content = page.locator('.is-message')
+const content = page.locator('[data-testid="modal-event-message"]')
examples/sites/demos/pc/app/modal/modal-header-composition-api.vue (3)

16-16: Consider removing redundant Modal alias

The explicit alias declaration on line 19 is redundant since Modal is already imported directly. This could be simplified by removing line 19 and using Modal directly throughout the code.

import { Button as TinyButton, Modal } from '@opentiny/vue'
import { ref } from 'vue'
-const TinyModal = Modal

Also applies to: 19-19


Line range hint 11-11: Consider updating template component name for consistency

While the script section uses Modal, the template still uses tiny-modal. Consider updating the template to maintain consistent naming:

-      <tiny-modal v-model="show" title="自定义弹窗标题" message="窗口内容" show-header show-footer> </tiny-modal>
+      <modal v-model="show" title="自定义弹窗标题" message="窗口内容" show-header show-footer> </modal>

Line range hint 1-45: Add missing e2e tests as mentioned in PR objectives

The PR objectives mention adding e2e tests, but no test file is included for this component. Please add corresponding e2e tests to validate the modal header functionality.

Would you like me to help generate a basic e2e test structure for this component? I can create a test file that covers:

  • Function-based modal header verification
  • Tag-based modal header verification
  • Custom title rendering
examples/sites/demos/pc/app/modal/modal-resize.spec.ts (2)

1-6: Enhance error handling and navigation robustness.

The current error handling could be improved to provide more detailed error information for debugging purposes.

Consider applying these improvements:

 import { test, expect } from '@playwright/test'

 test('弹窗调整大小', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+  page.on('pageerror', (exception) => {
+    console.error('Page Error:', exception);
+    expect(exception).toBeNull();
+  });
-  await page.goto('modal#modal-resize')
+  await page.goto('modal#modal-resize', { waitUntil: 'networkidle' });

1-25: Consider adding more comprehensive test coverage.

While the basic resize functionality is covered, consider adding these additional test cases:

  1. Keyboard-based resizing for accessibility (using arrow keys)
  2. Verification of resize handles visibility and styling
  3. Edge cases like minimum/maximum size constraints
  4. Double-click behavior on resize handles
  5. Touch-based resizing simulation

Would you like me to help create these additional test cases?

examples/sites/demos/pc/app/modal/modal-header.vue (2)

20-21: Consider aligning component names with their imports.

The components are imported as Button and Modal but registered as TinyButton and TinyModal. This creates a disconnect between the import names and their usage.

Consider updating the template and component registration to match:

-    TinyButton: Button,
-    TinyModal: Modal
+    Button,
+    Modal

Then update the template:

-    <tiny-button @click="btnClick">自定义弹窗标题</tiny-button>
+    <button @click="btnClick">自定义弹窗标题</button>
-    <tiny-modal v-model="show" title="自定义弹窗标题" message="窗口内容" show-header show-footer>
+    <modal v-model="show" title="自定义弹窗标题" message="窗口内容" show-header show-footer>

Line range hint 1-15: Consider adding TypeScript type definitions for props.

While the template demonstrates both functional and template-based approaches effectively, adding type definitions would improve maintainability and developer experience.

Consider converting to TypeScript and adding prop validations:

interface Props {
  show: boolean
  title?: string
  message?: string
  showHeader?: boolean
  showFooter?: boolean
}
examples/sites/demos/pc/app/modal/message-event.vue (3)

17-18: Remove eslint-disable comment by using arrow functions

The eslint-disable comment can be avoided by using arrow functions which preserve the this context.

- // eslint-disable-next-line @typescript-eslint/no-this-alias
- const $this = this

Line range hint 19-32: Simplify event handling using arrow functions

The event handling can be simplified by using arrow functions, which will maintain the correct this context.

Modal.message({
  message: '自定义消息的内容',
  events: {
-   show(ev) {
-     $this.handleEvent('show', ev)
-   },
-   hide(ev) {
-     $this.handleEvent('hide', ev)
-   },
-   close(ev) {
-     $this.handleEvent('close', ev)
-   }
+   show: (ev) => this.handleEvent('show', ev),
+   hide: (ev) => this.handleEvent('hide', ev),
+   close: (ev) => this.handleEvent('close', ev)
  }
})

35-37: Consider removing or conditionally including console.log

The console.log statement should not be present in production code. Consider using a proper logging system or removing it.

handleEvent(eventName, eventArgs) {
  Modal.message({ status: 'info', message: `${eventName} 事件触发了` })
- console.log({ eventName, eventArgs })
+ if (process.env.NODE_ENV !== 'production') {
+   console.log({ eventName, eventArgs })
+ }
}
examples/sites/demos/pc/app/modal/modal-mask-composition-api.vue (2)

28-32: Consider removing the TinyModal compatibility assignment

The assignment const TinyModal = Modal appears to be a compatibility layer. Since we're moving to the new Modal component, this assignment might create technical debt.

Consider removing this assignment and updating the template to use Modal directly:

- const TinyModal = Modal
  const show = ref(false)
  const options = ref({
    mask: true,
    maskClosable: true
  })

Also update the template:

- <tiny-modal
+ <modal
    v-model="show"
    type="confirm"
    title="自定义弹窗标题"
    message="窗口内容"
    show-header
    show-footer
    v-bind="options"
- </tiny-modal>
+ </modal>

Line range hint 36-42: LGTM! Consider adding TypeScript for better type safety

The Modal.confirm implementation looks correct and maintains all necessary options. However, since this is a crucial UI interaction point, consider adding TypeScript to ensure type safety for the configuration object.

Example TypeScript enhancement:

interface ModalOptions {
  message: string;
  title: string;
  showHeader: boolean;
  showFooter: boolean;
  mask?: boolean;
  maskClosable?: boolean;
}

function btnClick() {
  Modal.confirm({
    message: '窗口内容',
    title: '自定义弹窗标题',
    showHeader: true,
    showFooter: true,
    ...options.value
  } as ModalOptions)
}
examples/sites/demos/pc/app/modal/modal-resize-composition-api.vue (2)

25-25: Consider removing the redundant Modal alias

The alias TinyModal = Modal appears unnecessary since we're already importing Modal directly. This could potentially cause confusion for maintainers.

import { Button as TinyButton, Modal } from '@opentiny/vue'
import { ref } from 'vue'

-const TinyModal = Modal

Then update the template to use Modal instead of tiny-modal:

-      <tiny-modal
+      <modal
        v-model="show"
        type="confirm"
        title="自定义弹窗标题"
        message="窗口内容"
        show-header
        show-footer
        v-bind="options"
-      </tiny-modal>
+      </modal>

Also applies to: 28-28


Based on the test file content, I can now provide a final response:

E2e test coverage exists but needs enhancement

The existing e2e test modal-resize.spec.ts only covers the function-based modal resize scenario. The test should be extended to also verify the tag-based modal resize functionality shown in your component.

  • Add a new test case in modal-resize.spec.ts to verify the tag-based modal resize scenario using <tiny-modal> component
  • Test should verify both resize interactions and final dimensions for the tag-based implementation
🔗 Analysis chain

Line range hint 1-23: Verify e2e test coverage for modal resize functionality

The PR objectives mention adding e2e tests, but they're not visible in this file. Please ensure that e2e tests cover both function-based and tag-based modal resize scenarios.


Let me analyze the results and generate additional verification scripts to check the e2e test coverage for modal resize functionality.

The first command will show us the content of the identified modal resize test file to verify if it covers both function-based and tag-based scenarios. The second command will help us find any additional test files specifically related to modal resize functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related e2e test files
fd -e spec.js -e spec.ts -e test.js -e test.ts "modal.*resize"

# Look for existing modal e2e tests for reference
rg -l "test|describe|it.*modal.*resize" --type ts --type js

Length of output: 97941


Script:

#!/bin/bash
# Check content of the modal resize e2e test file
cat examples/sites/demos/pc/app/modal/modal-resize.spec.ts

# Look for any additional modal resize test files
rg -l "test.*modal.*resize" --type ts --type js examples/sites/demos/pc/app/modal/

Length of output: 938

examples/sites/demos/pc/app/modal/modal-mask.vue (1)

29-30: Consider simplifying component naming

While the current approach maintains backward compatibility, consider updating the local component names to match the imported ones for consistency:

-    TinyButton: Button,
-    TinyModal: Modal
+    Button,
+    Modal

This would require updating the template to use <Button> and <Modal> instead of <tiny-button> and <tiny-modal>, but would make the code more maintainable.

examples/sites/demos/pc/app/modal/modal-resize.vue (2)

29-30: Consider updating template tags to match new component names.

While the component registration maintains backward compatibility, it creates an inconsistency between imports and template usage. Consider updating the template to use the new component names directly.

- <tiny-button @click="btnClick">自定义弹窗调整大小</tiny-button>
+ <button @click="btnClick">自定义弹窗调整大小</button>

- <tiny-modal
+ <modal
    v-model="show"
    type="confirm"
    title="自定义弹窗标题"
    message="窗口内容"
    show-header
    show-footer
    v-bind="options"
  >
- </tiny-modal>
+ </modal>

And update component registration accordingly:

  components: {
-   TinyButton: Button,
-   TinyModal: Modal
+   Button,
+   Modal
  },

Several TinyModal.confirm usages need to be migrated to Modal.confirm

Found multiple files still using the old TinyModal.confirm API that need to be updated to use Modal.confirm. The changes should follow the same pattern as demonstrated in the modal-resize.vue example:

  • examples/sites/demos/pc/app/tree/edit-control.vue
  • examples/sites/demos/pc/app/tree/edit-control-composition-api.vue
  • examples/sites/demos/pc/app/tag/delete.vue
  • examples/sites/demos/pc/app/tag/delete-composition-api.vue
  • examples/sites/demos/pc/app/pager/before-page-change.vue
  • examples/sites/demos/pc/app/pager/before-page-change-composition-api.vue
  • examples/sites/demos/pc/app/pop-upload/* (multiple files)
  • examples/sites/demos/pc/app/file-upload/* (multiple files)
  • examples/sites/demos/mobile/app/modal/* (multiple files)
  • examples/sites/demos/mobile-first/app/modal/* (multiple files)
🔗 Analysis chain

Line range hint 47-53: LGTM! Verify similar Modal.confirm usages.

The Modal.confirm usage is correctly updated while maintaining all configuration options.

Let's verify similar usage patterns across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining TinyModal.confirm calls that should have been updated
rg "TinyModal\.confirm"

# Verify consistent usage of the new Modal.confirm pattern
ast-grep --pattern 'Modal.confirm({
  $$$
})'

Length of output: 4810

examples/sites/demos/pc/app/modal/basic-usage.spec.ts (2)

1-5: Consider enhancing error handling and navigation.

While the basic setup is good, there are opportunities for improvement:

  1. Make the error handler more specific:
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })
  1. Consider using a more robust URL navigation:
-  await page.goto('modal#basic-usage')
+  await page.goto('modal#basic-usage', { waitUntil: 'networkidle' })

3-33: Improve test organization with describe blocks.

Consider restructuring the tests for better organization and maintainability:

import { test, expect } from '@playwright/test'

test.describe('Modal Basic Usage', () => {
  test.beforeEach(async ({ page }) => {
    page.on('pageerror', (exception) => expect(exception).toBeNull())
    await page.goto('modal#basic-usage')
  })

  test('should handle basic prompt correctly', async ({ page }) => {
    await page.getByRole('button', { name: '基本提示框' }).click()
    // ... assertions
  })

  test('should display success prompt with correct styling', async ({ page }) => {
    await page.getByRole('button', { name: '成功提示框' }).click()
    // ... assertions
  })

  // ... other test cases
})
examples/sites/demos/pc/app/modal/modal-size-composition-api.vue (2)

33-39: Consider adding maximum dimension constraints

The modal options include minimum dimensions but lack maximum constraints. This could lead to UX issues on smaller screens.

const options = ref({
  width: 800,
  height: 400,
  minWidth: 700,
  minHeight: 300,
+ maxWidth: '90vw',
+ maxHeight: '90vh'
})

Line range hint 41-49: Add error handling for modal interactions

The modal confirmation lacks error handling. Consider adding try-catch blocks and handling potential rejection cases.

function btnClick(isFull) {
-  Modal.confirm({
-    message: '窗口内容',
-    title: '自定义弹窗标题',
-    showHeader: true,
-    showFooter: true,
-    ...options.value,
-    fullscreen: isFull
-  })
+  Modal.confirm({
+    message: '窗口内容',
+    title: '自定义弹窗标题',
+    showHeader: true,
+    showFooter: true,
+    ...options.value,
+    fullscreen: isFull
+  }).catch(error => {
+    console.error('Modal interaction failed:', error)
+    // Handle error appropriately
+  })
}
examples/sites/demos/pc/app/modal/modal-size.vue (1)

28-28: Consider updating template tags for consistency

While the component registration maintains backward compatibility by aliasing Button and Modal as TinyButton and TinyModal, it would be more maintainable to update the template tags to match the new component names.

- <tiny-button @click="btnClick(false)">自定义弹窗大小</tiny-button>
+ <button @click="btnClick(false)">自定义弹窗大小</button>

- <tiny-modal
+ <modal
    v-model="show"
    type="confirm"
    ...
- </tiny-modal>
+ </modal>

Also applies to: 32-33

examples/sites/demos/pc/app/modal/modal-fn-slots-composition-api.vue (1)

Line range hint 4-19: Consider internationalizing the Chinese text

The modal implementation looks good, but consider extracting the Chinese text into i18n translation files for better maintainability and internationalization support.

Example improvement:

- <tiny-button @click="btnClick">打开带插槽弹窗</tiny-button>
+ <tiny-button @click="btnClick">{{ t('modal.openSlotModal') }}</tiny-button>

- <tiny-button @click="openModal">打开带插槽弹窗</tiny-button>
+ <tiny-button @click="openModal">{{ t('modal.openSlotModal') }}</tiny-button>
examples/sites/demos/pc/app/modal/modal-other-composition-api.vue (1)

34-40: LGTM: Improved import clarity with named imports

The switch to named imports with explicit aliases improves code maintainability and follows modern JavaScript practices.

Consider grouping related components together in the import statement for better readability:

import {
  Button as TinyButton,
  Form as TinyForm,
  FormItem as TinyFormItem,
  Input as TinyInput,
  Modal
} from '@opentiny/vue'
examples/sites/demos/pc/app/modal/modal-fn-slots.vue (1)

27-29: Consider aligning component registration names with imports

The components are imported without the 'Tiny' prefix but registered with it. This creates an inconsistency between imports and usage. Consider either:

  1. Using the non-prefixed names consistently throughout
  2. Adding the prefix during import instead of registration
-    TinyButton: Button,
-    TinyModal: Modal,
-    TinyLink: Link
+    Button,
+    Modal,
+    Link

Then update template usage accordingly:

-    <tiny-button @click="btnClick">
+    <Button @click="btnClick">
examples/sites/demos/pc/app/modal/modal-event-composition-api.vue (1)

31-31: Remove unnecessary Modal alias

The TinyModal alias is no longer needed since the code has been updated to use Modal directly. Consider removing line 34 to improve code clarity.

import { Button as TinyButton, Modal } from '@opentiny/vue'
import { ref } from 'vue'
-const TinyModal = Modal

Also applies to: 34-34

examples/sites/demos/pc/app/modal/modal-other.vue (1)

38-42: Consider future template updates for consistency

The component registration maintains backward compatibility while allowing for the new import style. However, consider updating template usage to match the new component names in a future PR for consistency.

Future template update example:

-<tiny-button @click="btnClick">
+<Button @click="btnClick">
examples/sites/demos/pc/app/modal/modal-lock-composition-api.vue (3)

32-32: Consider removing redundant Modal alias

The constant TinyModal appears to be an unnecessary alias for Modal. This could cause confusion and make the code harder to maintain.

-import { Button as TinyButton, Modal } from '@opentiny/vue'
+import { Button as TinyButton, Modal as TinyModal } from '@opentiny/vue'

-const TinyModal = Modal

Also applies to: 35-35


Line range hint 49-60: Add parameter validation for lockType

The lockType parameter should be validated to ensure it only accepts valid values ('scroll' or 'view').

 function btnClick(lockType) {
+  if (lockType !== 'scroll' && lockType !== 'view') {
+    throw new Error('lockType must be either "scroll" or "view"')
+  }
   options.value.lockScroll = lockType === 'scroll'
   options.value.lockView = lockType === 'view'

   Modal.confirm({

Line range hint 62-66: Extract common lock type handling logic

The lock type handling logic is duplicated between btnClick and openModal. Consider extracting it into a shared function.

+function setLockOptions(lockType) {
+  if (lockType !== 'scroll' && lockType !== 'view') {
+    throw new Error('lockType must be either "scroll" or "view"')
+  }
+  options.value.lockScroll = lockType === 'scroll'
+  options.value.lockView = lockType === 'view'
+}

 function btnClick(lockType) {
-  options.value.lockScroll = lockType === 'scroll'
-  options.value.lockView = lockType === 'view'
+  setLockOptions(lockType)
   Modal.confirm({
     message: '窗口内容',
     title: '自定义弹窗标题',
     showHeader: true,
     showFooter: true,
     ...options.value
   })
 }

 function openModal(lockType) {
-  options.value.lockScroll = lockType === 'scroll'
-  options.value.lockView = lockType === 'view'
+  setLockOptions(lockType)
   show.value = true
 }
examples/sites/demos/pc/app/modal/basic-usage-composition-api.vue (3)

27-27: Consider removing redundant Modal reassignment

The const reassignment TinyModal = Modal appears unnecessary since the template still uses tiny-modal kebab-case notation. This reassignment could cause confusion during maintenance.

 import { Button as TinyButton, Modal, Notify } from '@opentiny/vue'
 import { ref } from 'vue'
-const TinyModal = Modal

Also applies to: 30-30


44-50: Add error handling to confirm dialog

The promise chain should include error handling for cases where the confirmation fails or is rejected.

 Modal.confirm('您确定要删除吗?').then((res) => {
   Notify({
     type: 'info',
     title: '触发回调事件',
     message: `点击${res}按钮`
   })
+ }).catch(error => {
+   Notify({
+     type: 'error',
+     title: '操作失败',
+     message: '确认对话框发生错误'
+   })
 })

53-62: Enhance JSX button accessibility

The button in JSX content lacks proper accessibility attributes and styling consistency with the rest of the application.

 message: (
   <div>
-    <button> some button </button>
+    <button
+      role="button"
+      class="tiny-button"
+      aria-label="Action button"
+    >
+      some button
+    </button>
     <b> some text </b>
   </div>
 ),
examples/sites/demos/pc/app/modal/modal-footer.vue (3)

43-43: Simplify component registration

Since you're importing the base component names, you can simplify the component registration by using them directly instead of aliasing with the Tiny prefix.

-  components: {
-    TinyButton: Button,
-    TinyModal: Modal,
-    TinyLink: Link
-  },
+  components: {
+    Button,
+    Modal,
+    Link
+  },

Then update the template to use the base component names:

-      <tiny-button @click="btnClick">
+      <Button @click="btnClick">

Also applies to: 47-49


Line range hint 66-72: Fix property spelling in options object

The footerDragable property in the options object should be renamed to footerDraggable for consistency and correct spelling.

       options: {
         confirmContent: 'Okk~~',
         cancelContent: 'No thanks!',
         confirmBtnProps: { type: 'info' },
         cancelBtnProps: { type: 'success' },
-        footerDragable: true
+        footerDraggable: true
       }

E2e test coverage needs to be extended for footer dragging

While e2e tests exist for the modal footer in modal-footer.spec.ts, they only cover the custom footer content verification. The tests do not include scenarios for testing the footer dragging functionality (footer-dragable prop) which is an important feature mentioned in the code.

Key missing test scenarios:

  • Footer dragging initialization with footer-dragable prop
  • Drag interaction behavior on the footer
  • Verification of CSS classes for draggable state
🔗 Analysis chain

Line range hint 1-1: Verify e2e test coverage

The PR objectives mention adding e2e tests, but no tests are visible in this file. Please ensure that appropriate e2e tests have been added to verify the modal footer dragging functionality.


Let me check the content of the e2e test file found to verify if it covers the footer dragging functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related e2e test files
fd -e spec.ts -e spec.js "modal.*footer"

# Search for test cases related to footer dragging
rg -i "drag.*footer|footer.*drag" -t ts -t js

Length of output: 1374


Script:

#!/bin/bash
# Check content of the e2e test file
cat examples/sites/demos/pc/app/modal/modal-footer.spec.ts

Length of output: 621

examples/sites/demos/pc/app/modal/basic-usage.vue (2)

27-27: Maintain consistent component naming

The components are imported without the 'Tiny' prefix but registered with it. This creates unnecessary aliasing. Consider using consistent naming throughout the component.

 import { Button, Modal, Notify } from '@opentiny/vue'

 export default {
   components: {
-    TinyButton: Button,
-    TinyModal: Modal
+    Button,
+    Modal
   },

Then update the template to use <Button> and <Modal> instead of <tiny-button> and <tiny-modal>.

Also applies to: 31-32


58-67: Extract JSX content to improve maintainability

Consider extracting the JSX content into a separate method or computed property for better maintainability and reusability.

 export default {
+  computed: {
+    modalContent() {
+      return (
+        <div>
+          <button>some button</button>
+          <b>some text</b>
+        </div>
+      )
+    }
+  },
   methods: {
     jsxClick() {
       Modal.alert({
-        message: (
-          <div>
-            <button>some button</button>
-            <b>some text</b>
-          </div>
-        ),
+        message: this.modalContent,
         status: 'success'
       })
     }
   }
 }
examples/sites/demos/pc/app/modal/modal-event.vue (2)

Line range hint 47-73: Simplify scope handling with arrow functions

The current implementation uses a $this variable with an eslint disable comment. This can be simplified using arrow functions:

    btnClick() {
-     // eslint-disable-next-line @typescript-eslint/no-this-alias
-     const $this = this
      Modal.confirm({
        message: '窗口内容',
        title: '自定义弹窗标题',
        showHeader: true,
        showFooter: true,
        resize: true,
        events: {
-         show(ev) {
-           $this.handleEvent('show', ev)
-         },
+         show: (ev) => this.handleEvent('show', ev),
          // Apply similar changes to other event handlers
        }
      })
    },

80-82: Consider removing or documenting console.log

The console.log statement might be intentional for demo purposes. If this is a demo file, consider adding a comment explaining its purpose. If not, consider removing it or replacing with proper logging.

    handleEvent(eventName, eventArgs) {
      Modal.message({ status: 'info', message: `${eventName} 事件触发了` })
+     // Demo: Log event details for demonstration purposes
      console.log({ eventName, eventArgs })
    }
examples/sites/demos/pc/app/modal/status.spec.ts (3)

1-8: Consider improving test setup robustness

A few suggestions to enhance the test reliability:

  1. Use English test names for better CI/CD compatibility
  2. Add more specific error handling
  3. Use data-testid attributes for more reliable element selection
-test('状态和图标', async ({ page }) => {
+test('modal status icons and messages', async ({ page }) => {
   // Add specific error handling
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })
   await page.goto('modal#status')
-  const modal = page.locator('.tiny-modal__status-wrapper svg').first()
-  const message = page.locator('.type__message')
+  const modal = page.locator('[data-testid="modal-status-icon"]').first()
+  const message = page.locator('[data-testid="message-status"]')

9-15: Enhance test maintainability for info icon tests

The current implementation uses nth selectors and button text for element selection, which can be fragile if the UI changes.

-  // 信息提示图标
+  // Info icon modal test
-  await page.getByRole('button', { name: '信息提示图标' }).first().click()
+  await page.locator('[data-testid="info-modal-trigger"]').click()
   await expect(modal).toHaveClass(/tiny-modal-svg__info/)
   await page.getByRole('button', { name: '确定' }).click()
-  // ----消息状态示例----
+  // Info icon message test
-  await page.getByRole('button', { name: '信息提示图标' }).nth(1).click()
+  await page.locator('[data-testid="info-message-trigger"]').click()
   await expect(message).toHaveClass(/status__info/)

1-48: Add tests for accessibility and animations

Important test scenarios are missing:

  1. Accessibility testing (ARIA attributes, focus management)
  2. Animation timing and transitions
  3. Modal backdrop interaction

Consider adding these test cases:

// Accessibility tests
test('should manage focus correctly', async ({ page }) => {
  // Test focus trap inside modal
  // Test focus restoration after modal closes
})

// Animation tests
test('should animate modal correctly', async ({ page }) => {
  // Test animation classes
  // Test transition timing
})

// Backdrop tests
test('should handle backdrop clicks', async ({ page }) => {
  // Test modal closure on backdrop click
  // Test modal persistence when closeOnClickModal is false
})
examples/sites/demos/pc/app/modal/status-composition-api.vue (1)

32-37: Remove unnecessary Modal alias

The const TinyModal = Modal assignment appears to be redundant since:

  1. Modal is already imported directly
  2. The template uses tiny-modal which is likely registered globally
  3. All other code uses Modal directly
import { Button as TinyButton, Modal } from '@opentiny/vue'
import { ref } from 'vue'

-const TinyModal = Modal
const show = ref(false)
const status = ref('')
examples/sites/demos/pc/app/modal/status.vue (2)

47-80: Consider refactoring click handlers to reduce duplication

The click handlers follow identical patterns with only the status and message text changing. This could be refactored into a single method for better maintainability.

Consider refactoring like this:

+ const STATUS_CONFIGS = {
+   info: { message: '基本提示图标', status: 'info' },
+   success: { message: '成功提示图标', status: 'success' },
+   warning: { message: '警告提示图标', status: 'warning' },
+   error: { message: '错误提示图标', status: 'error' },
+   loading: { message: '加载提示图标', status: 'loading' }
+ }
+
+ methods: {
+   showModal(type, mode) {
+     const config = STATUS_CONFIGS[type]
+     if (mode === 1) {
+       Modal.alert(config)
+     } else if (mode === 2) {
+       Modal.message(config)
+     } else if (mode === 3) {
+       this.status = config.status
+       this.show = true
+     }
+   }
+ }

Then update the template:

- <tiny-button @click="infoClick(1)">
+ <Button @click="showModal('info', 1)">

47-48: Consider adding type safety for modal configurations

The modal configurations would benefit from TypeScript interfaces to ensure type safety and provide better IDE support.

Consider adding these TypeScript definitions:

interface ModalConfig {
  message: string;
  status: 'info' | 'success' | 'warning' | 'error' | 'loading';
}

interface ModalMethods {
  alert(config: ModalConfig): void;
  message(config: ModalConfig): void;
}

Also applies to: 55-56, 63-64, 71-72, 79-80

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1df0f4d and 2067a96.

📒 Files selected for processing (50)
  • examples/sites/demos/pc/app/modal/basic-usage-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/basic-usage.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/basic-usage.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/footer-slot-composition-api.vue (0 hunks)
  • examples/sites/demos/pc/app/modal/footer-slot.vue (0 hunks)
  • examples/sites/demos/pc/app/modal/message-closable-composition-api.vue (0 hunks)
  • examples/sites/demos/pc/app/modal/message-closable.vue (0 hunks)
  • examples/sites/demos/pc/app/modal/message-close-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/message-close.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/message-close.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/message-event-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/message-event.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/message-event.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/message-id-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/message-id.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/message-id.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/message-top-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/message-top.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/message-top.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-event-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-event.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-event.vue (3 hunks)
  • examples/sites/demos/pc/app/modal/modal-fn-slots-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-fn-slots.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-fn-slots.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-footer-composition-api.vue (3 hunks)
  • examples/sites/demos/pc/app/modal/modal-footer.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-footer.vue (3 hunks)
  • examples/sites/demos/pc/app/modal/modal-header-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-header.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-header.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-lock-composition-api.vue (3 hunks)
  • examples/sites/demos/pc/app/modal/modal-lock.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-lock.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-mask-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-mask.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-mask.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-other-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-other.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-other.vue (3 hunks)
  • examples/sites/demos/pc/app/modal/modal-resize-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-resize.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-resize.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/modal-size-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-size.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/modal-size.vue (2 hunks)
  • examples/sites/demos/pc/app/modal/status-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/modal/status.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/modal/status.vue (2 hunks)
  • packages/renderless/src/modal/index.ts (1 hunks)
💤 Files with no reviewable changes (4)
  • examples/sites/demos/pc/app/modal/footer-slot-composition-api.vue
  • examples/sites/demos/pc/app/modal/footer-slot.vue
  • examples/sites/demos/pc/app/modal/message-closable-composition-api.vue
  • examples/sites/demos/pc/app/modal/message-closable.vue
🔇 Additional comments (70)
packages/renderless/src/modal/index.ts (1)

307-308: Ensures modal reverts to original state when not in fullscreen mode

Calling api.revert() in the open method when props.fullscreen is false ensures that the modal resets to its original size and position if it was previously maximized. This maintains consistency and correct behavior when opening the modal.

examples/sites/demos/pc/app/modal/message-id.spec.ts (1)

1-2: LGTM! Clean and minimal imports.

The necessary Playwright test utilities are correctly imported.

examples/sites/demos/pc/app/modal/message-close.spec.ts (2)

1-2: LGTM!

The imports are clean and appropriate for a Playwright test file.


5-8: 🛠️ Refactor suggestion

Enhance test reliability and coverage

Several improvements could make this test more robust and comprehensive:

  1. The URL navigation seems incomplete
  2. The class selector might be fragile
  3. Missing verification of modal closing behavior

Consider these improvements:

-  await page.goto('modal#message-close')
-  const content = page.locator('.tiny-modal__text')
-  await page.getByRole('button', { name: '消息可关闭' }).click()
-  await expect(content).toHaveText(/5s 后得自动关闭/)
+  await page.goto('modal#message-close', { waitUntil: 'networkidle' })
+  
+  // Use more reliable selectors
+  const modal = page.getByRole('dialog')
+  const content = modal.getByRole('textContent')
+  
+  // Click and verify modal appears
+  await page.getByRole('button', { name: '消息可关闭' }).click()
+  await expect(modal).toBeVisible()
+  await expect(content).toHaveText('5s 后得自动关闭')
+  
+  // Verify modal closes
+  await expect(modal).toBeHidden({ timeout: 6000 })

Let's verify the URL structure and available selectors:

✅ Verification successful

Based on the search results, I can see that .tiny-modal__text is a standard class used across the codebase for modal text content. The URL pattern modal#message-close is also consistent with other modal examples. The test is verifying a message modal's text content and auto-close behavior.

Let me verify the auto-close functionality:


Let me check the complete implementation of the message-close functionality:


Test correctly verifies modal behavior

The test is correctly verifying the modal's text content and matches the actual implementation. The code shows:

  1. The modal is configured with duration: 5000 (5 seconds) for auto-close
  2. The message text in the implementation matches what the test is looking for: "5s 后得自动关闭"
  3. The .tiny-modal__text selector is the standard class used for modal text content across the codebase

The test appropriately:

  • Clicks the button that triggers the modal
  • Verifies the expected text content
  • Uses the correct selector
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other modal test files to understand URL patterns
fd -e spec.ts -e spec.js . | xargs rg "page\.goto.*modal.*"

# Check for consistent selector usage across modal tests
rg "tiny-modal" -A 2 -B 2

Length of output: 102696


Script:

#!/bin/bash
# Check the message-close implementation to verify auto-close behavior
rg -A 10 "消息可关闭" examples/sites/demos/pc/app/modal/

# Check for any timeout/auto-close related code
rg "setTimeout.*modal" examples/sites/demos/pc/app/modal/

Length of output: 2408


Script:

#!/bin/bash
# Get the full implementation of message-close
cat examples/sites/demos/pc/app/modal/message-close.vue
cat examples/sites/demos/pc/app/modal/message-close-composition-api.vue

Length of output: 1064

examples/sites/demos/pc/app/modal/message-top.spec.ts (2)

1-2: LGTM!

The imports are clean and include only the necessary testing utilities from Playwright.


3-9: Verify test coverage completeness

Let's ensure we have comprehensive test coverage for the modal positioning feature.

✅ Verification successful

Test coverage is appropriate for the modal top positioning feature

The test coverage is sufficient because:

  1. The feature being tested is a specific, isolated functionality (setting modal message top position to 500px)
  2. The implementation files show this is a simple feature where the top property is directly passed to the Modal component
  3. The test verifies the exact CSS property that corresponds to the feature
  4. No other positioning-related tests are needed as this is the only top positioning feature in the modal messages
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related modal position tests
echo "Searching for related modal position tests..."
rg -t ts "test\(.*top.*modal" examples/sites/demos/pc/app/modal/
rg -t ts "test\(.*modal.*position" examples/sites/demos/pc/app/modal/

# Search for the implementation of the top positioning feature
echo "Searching for top positioning implementation..."
rg -t vue "消息距离顶部" examples/sites/demos/pc/app/modal/

Length of output: 495


Script:

#!/bin/bash
# Search for related modal position tests with broader patterns
echo "Searching for related modal position tests..."
rg -t typescript "test.*top" examples/sites/demos/pc/app/modal/
rg -t typescript "test.*position" examples/sites/demos/pc/app/modal/

# Search for the implementation with correct file extension
echo "Searching for top positioning implementation..."
fd . examples/sites/demos/pc/app/modal/ -e vue -x rg -l "消息距离顶部"

# List all test files in the modal directory to understand test coverage
echo "Listing all test files..."
fd . examples/sites/demos/pc/app/modal/ -e spec.ts

Length of output: 1587


Script:

#!/bin/bash
# Let's check the content of both implementation files and the test file
echo "Checking message-top implementation files..."
cat examples/sites/demos/pc/app/modal/message-top.vue
cat examples/sites/demos/pc/app/modal/message-top-composition-api.vue

echo "Checking message-top test file..."
cat examples/sites/demos/pc/app/modal/message-top.spec.ts

# Let's also check if there are any other tests related to positioning
echo "Searching for positioning-related tests..."
rg "toHaveCSS.*top|toHaveCSS.*position" examples/sites/demos/pc/app/modal/

Length of output: 1869

examples/sites/demos/pc/app/modal/message-top-composition-api.vue (2)

10-10: LGTM! Import statement follows modern Vue practices.

The updated import statement correctly uses named imports and proper aliasing.


Line range hint 13-17: Verify Modal.message API parameters.

The implementation looks correct, but let's verify that the Modal.message API supports these parameters consistently across the codebase.

✅ Verification successful

Based on the extensive search results, I can now provide a definitive response about the Modal.message API usage:

The Modal.message API parameters are correctly implemented and consistent across the codebase.

The search results show consistent usage patterns for Modal.message across the codebase with the following supported parameters:

  • message: The message content (required)
  • status: Message type ('info', 'success', 'warning', 'error', 'loading')
  • top: Distance from the top of the viewport (optional)
  • id: Unique identifier to prevent duplicate messages (optional)
  • duration: Auto-close duration in milliseconds (optional)
  • events: Event handlers object (optional)

The implementation in the review matches these standard parameters, particularly using status, message and top which are all valid and commonly used throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Modal.message API usage patterns across the codebase

# Search for other Modal.message usages to confirm parameter consistency
echo "Searching for Modal.message usage patterns..."
rg -A 3 "Modal\.message\(\{" --type vue

# Search for any documentation or type definitions
echo "Searching for Modal API documentation or types..."
fd -e ts -e js -e d.ts --exec rg -l "Modal\.message"

Length of output: 791


Script:

#!/bin/bash
# Let's try a different approach to search for Modal.message usage

# Search for Modal.message in all files, including Vue files
echo "Searching for Modal.message usage patterns..."
rg -t vue -t js -t ts "Modal\.message\(" -A 3

# Search specifically for Modal.message type definitions or interfaces
echo "Searching for Modal type definitions..."
rg "interface.*Modal|type.*Modal|Modal\.message" --type typescript -A 5

Length of output: 374


Script:

#!/bin/bash
# Let's try one more approach without file type restrictions

# Search for Modal.message usage across all files
echo "Searching for Modal.message usage..."
rg "Modal\.message\(" -A 3

# Search for files containing Modal-related definitions
echo "Searching for Modal-related files..."
fd -t f -e ts -e js -e vue Modal

# Try to find any documentation files
echo "Searching for documentation..."
fd README -e md | xargs rg -l "Modal"

Length of output: 132658

examples/sites/demos/pc/app/modal/message-id-composition-api.vue (2)

10-10: LGTM! Verify consistent Modal import pattern.

The import statement correctly uses the new Modal component name while maintaining backward compatibility for Button.

Let's verify the consistency of Modal imports across the codebase:


Line range hint 13-17: Add JSDoc and ensure test coverage for unique ID feature.

The Modal.message implementation with unique ID is a good practice to prevent duplicate modals. Consider adding documentation to explain this behavior.

Add JSDoc comment above the function:

+/**
+ * Demonstrates Modal.message with unique ID to prevent duplicate messages
+ * when button is clicked multiple times.
+ */
function btnClick() {
  Modal.message({
    status: 'info',
    message: '自定义消息具有唯一 id,所以不会重复出现',
    id: 'unique'
  })
}

Let's verify the test coverage for this feature:

✅ Verification successful

Based on the search results, I can now generate the final response:

Test coverage exists for Modal.message with unique ID feature

The test coverage for Modal.message with unique ID exists in packages/vue/src/modal/__tests__/modal.test.tsx:

test('id', async () => {
  const handleClick = async () => {
    Modal.message({
      id: 'unique',
      message: 'no repeat'
    })
  }
})

The implementation is also widely used across the codebase in various components, demonstrating its stability and reliability. No additional JSDoc is needed since this is a demo file showcasing the feature.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing test coverage of Modal.message with unique ID

# Search for related test files
rg -l "Modal.message.*unique" --type ts --glob "**/modal/**/*.spec.ts"

# Search for test cases covering duplicate message prevention
rg "test.*duplicate.*message" --type ts --glob "**/modal/**/*.spec.ts"

Length of output: 145


Script:

#!/bin/bash
# Let's broaden our search to find any Modal message-related tests and implementation

# Search for Modal message tests with broader patterns
rg "Modal.*message" --type ts --glob "**/test/**/*.spec.ts"
rg "Modal.*message" --type ts --glob "**/tests/**/*.spec.ts"

# Look for the Modal implementation to understand the message handling
ast-grep --pattern 'class Modal {
  $$$
  message($$$) {
    $$$
  }
  $$$
}'

# Search for any documentation about Modal message functionality
rg -l "Modal.*message" --type md

Length of output: 634


Script:

#!/bin/bash
# Let's check the documentation content and adjust our search patterns

# Check the content of the modal documentation
cat examples/sites/demos/pc/app/modal/webdoc/modal.en.md

# Try different search patterns for tests
rg "Modal" --type ts --glob "**/*.spec.{ts,js}"
rg "Modal" --type vue --glob "**/modal/**/*.vue"

# Look for the Modal implementation with a different pattern
ast-grep --pattern 'message($$$) {
  $$$
}'

Length of output: 6925


Script:

#!/bin/bash
# Let's try one final search to find the Modal implementation and related tests

# Search for any .vue files in modal directory
fd -e vue -p "**/modal/"

# Search for Modal implementation in source files
rg -t js -t ts "Modal.message" -B 2 -A 2

# Check changelog for any mentions of Modal message feature
cat examples/sites/demos/pc/webdoc/changelog.md

Length of output: 126842

examples/sites/demos/pc/app/modal/message-close-composition-api.vue (1)

10-10: LGTM: Import statement updates are consistent

The change from TinyModal to Modal aligns with the modernization effort while maintaining backward compatibility through the Button alias.

examples/sites/demos/pc/app/modal/message-event.spec.ts (1)

1-2: LGTM: Imports are clean and appropriate

The necessary test utilities are imported from Playwright.

examples/sites/demos/pc/app/modal/modal-other.spec.ts (2)

1-2: LGTM!

The imports are correctly specified for Playwright test framework.


5-5: Verify the navigation URL format

The URL format using fragment identifier needs verification to ensure it's consistent with the application's routing structure.

✅ Verification successful

Based on the search results, I can see that fragment identifier URLs (using #) are consistently used across the entire test suite for navigation. The pattern goto('component-name#demo-name') is used extensively throughout the codebase's test files, showing this is the standard URL format for the application's routing structure.

URL format is consistent with codebase patterns

The navigation URL format modal#modal-other follows the established pattern used throughout the test suite, where component routes use fragment identifiers to specify different demo sections.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for similar URL patterns in other test files
# Expected: Find similar navigation patterns to confirm URL format consistency

# Search for goto calls with fragment identifiers in test files
rg "goto\(['\"].*#.*['\"]" -t ts

Length of output: 121343

examples/sites/demos/pc/app/modal/modal-header.spec.ts (2)

1-2: LGTM: Imports are correct and minimal

The test imports exactly what's needed from the Playwright test framework.


1-12: Verify test coverage across different scenarios

While the basic functionality is tested, consider adding tests for:

  1. Modal with very long titles
  2. Modal with HTML content in title
  3. Modal with empty title
  4. Modal title updates
✅ Verification successful

Let me search for the modal component's test files and implementation to better understand the test coverage.


Let me check the modal component's implementation to understand how it handles titles.


Let me try a different approach to find the modal component's implementation and test files.


Let me check the modal component's test file and implementation.


Based on the comprehensive test file and implementation code review, I can now provide a final response:

Test coverage for modal titles is sufficient

The current test coverage adequately verifies modal title functionality:

  1. Basic title rendering is tested in the main test file (test('title'))
  2. Title with HTML content is handled through the component's sanitization
  3. Empty title scenario is covered by show-header test case
  4. Title updates are implicitly tested through v-model and state management tests

The test file shows thorough coverage of modal functionality including:

  • Title rendering and visibility
  • Header show/hide behavior
  • Modal state management
  • Event handling
  • Various modal types (alert, confirm, message)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing test coverage of these scenarios
rg -A 5 "test\(.*modal.*title" || \
rg -A 5 "test\(.*标题"

Length of output: 8273


Script:

#!/bin/bash
# Search for modal test files
fd "modal.*spec" examples/sites/demos/pc/app/modal/

# Search for modal title related test cases
rg -A 10 "modal.*title|标题" examples/sites/demos/pc/app/modal/

Length of output: 45421


Script:

#!/bin/bash
# Search for modal component implementation
ast-grep --pattern 'class Modal {
  $$$
}'

# Search for title-related code in modal files
rg -g "modal*" "title" src/

Length of output: 117


Script:

#!/bin/bash
# Find all modal-related files in the repository
fd -t f "modal" 

# Search for title-related test cases in all spec files
rg -t spec "test.*title|test.*标题" 

Length of output: 3607


Script:

#!/bin/bash
# Check the main modal test file
cat packages/vue/src/modal/__tests__/modal.test.tsx

# Check the modal component implementation
cat packages/mobile/components/modal/src/modal.ts

Length of output: 17437

examples/sites/demos/pc/app/modal/message-top.vue (2)

Line range hint 18-22: Verify Modal API compatibility and consider using constants

The change from TinyModal.message to Modal.message looks correct, but two suggestions:

  1. Let's verify the new Modal API maintains backward compatibility
  2. Consider extracting magic numbers and strings into constants

Consider this refactor:

+const MODAL_CONFIG = {
+  TOP_OFFSET: 500,
+  STATUS: {
+    INFO: 'info'
+  },
+  MESSAGES: {
+    CUSTOM_TOP: '自定义消息的内容距离顶部500px'
+  }
+}
+
 btnClick() {
   Modal.message({
-    status: 'info',
-    message: '自定义消息的内容距离顶部500px',
-    top: 500
+    status: MODAL_CONFIG.STATUS.INFO,
+    message: MODAL_CONFIG.MESSAGES.CUSTOM_TOP,
+    top: MODAL_CONFIG.TOP_OFFSET
   })
 }
✅ Verification successful

Based on the search results, I can see extensive usage of both Modal.message() and TinyModal.message() APIs across the codebase. The key observations are:

  1. The Modal.message() API is used consistently with the same parameter structure as shown in the review code:
Modal.message({
  status: 'info',
  message: '...',
  top: 500
})
  1. The API accepts the same configuration options including status, message, and positioning properties like top.

  2. The change from TinyModal.message to Modal.message appears to be part of a broader API consolidation, with both APIs having identical parameter structures and behavior.

Given this evidence showing backward compatibility is maintained in the API signature and behavior, I can generate the final response:

API change verified - Modal.message maintains backward compatibility

The change from TinyModal.message to Modal.message is safe as both APIs use identical parameter structures and behavior. The codebase shows consistent usage of the same configuration options including status, message content, and positioning properties.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any breaking changes in Modal API across the codebase
rg -B2 -A2 "Modal\.message\(" 

Length of output: 139959


10-10: Verify documentation for the new import path

The change from TinyButton, TinyModal to Button, Modal appears correct, but let's ensure this new import pattern is documented.

✅ Verification successful

Let me check the content of these documentation files to verify if they properly document the new import pattern.


Import pattern is properly documented

The new import pattern import { Button, Modal } from '@opentiny/vue' is well documented across multiple documentation files. The documentation explicitly mentions that:

  • Since version 3.17.0, both prefixed (TinyButton) and unprefixed (Button) imports are supported
  • For versions before 3.17.0, aliases should be used: import { Button as TinyButton }
  • The import pattern is documented in both Chinese and English documentation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the new import pattern is documented in README or migration guides
fd -e md -e mdx | xargs rg -l "@opentiny/vue.*(?:Button|Modal)"

Length of output: 217


Script:

#!/bin/bash
# Check the content of the documentation files
echo "=== Content of import-components.md ==="
cat examples/sites/demos/pc/webdoc/import-components.md

echo -e "\n=== Content of import-components-en.md ==="
cat examples/sites/demos/pc/webdoc/import-components-en.md

echo -e "\n=== Content of mobile import.md ==="
cat examples/sites/demos/mobile/webdoc/import.md

Length of output: 9503

examples/sites/demos/pc/app/modal/message-id.vue (1)

10-10: Verify the impact of the component rename

The change from TinyModal/TinyButton to Modal/Button appears to be part of a broader refactoring effort.

✅ Verification successful

Based on the search results, I can see that there is a widespread refactoring effort across the codebase where components are being renamed from TinyModal/TinyButton to Modal/Button. The changes are consistent with the broader refactoring pattern, and the import changes in the file under review align with this pattern.

Component rename is part of a consistent refactoring

The change from TinyModal/TinyButton to Modal/Button in the imports is part of a broader, intentional refactoring effort across the codebase. This is evidenced by:

  • Similar import changes in many other files
  • Consistent pattern of importing from '@opentiny/vue'
  • Both old and new component names are being used in transition
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any remaining usages of the old component names
rg "TinyModal\.|TinyButton" --type vue

# Check if the new component names are consistently used
rg "Modal\.|Button" --type vue

Length of output: 129


Script:

#!/bin/bash
# Search for old component names (TinyModal/TinyButton) in all files
rg "TinyModal|TinyButton"

# Search for new component names (Modal/Button) in all files
rg "import.*\{ ?(?:Modal|Button).*\} from '@opentiny/vue'"

Length of output: 127784

examples/sites/demos/pc/app/modal/message-close.vue (1)

Line range hint 1-35: Verify consistency across modal demos

Let's ensure this refactoring pattern is consistently applied across all modal demos.

✅ Verification successful

Consistent modal usage pattern found across demos

The verification shows that the modal usage pattern in message-close.vue is consistent with other modal demos in the codebase:

  • All modal demos consistently use tiny-button component in templates
  • Modal imports follow a consistent pattern from '@opentiny/vue'
  • The TinyModal references found are just component aliases, which is a standard practice across the demos
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for inconsistent modal component usage across demos

# Check for remaining TinyModal usage
echo "Checking for remaining TinyModal usage in demos:"
rg "TinyModal" "examples/sites/demos/pc/app/modal"

# Check for inconsistent button component usage in templates
echo "Checking for inconsistent button usage in templates:"
rg "<tiny-button" "examples/sites/demos/pc/app/modal"

# Check import patterns
echo "Checking modal import patterns:"
ast-grep --pattern 'import { $$$, Modal, $$$ } from "@opentiny/vue"' "examples/sites/demos/pc/app/modal"

Length of output: 15000

examples/sites/demos/pc/app/modal/modal-footer.spec.ts (1)

1-2: LGTM: Imports are clean and appropriate

The necessary Playwright testing utilities are correctly imported.

examples/sites/demos/pc/app/modal/modal-mask.spec.ts (3)

1-2: LGTM!

The necessary Playwright test utilities are correctly imported.


7-13: 🛠️ Refactor suggestion

Several improvements needed for test robustness.

The current test implementation has several areas that could be improved:

  1. Using nth selectors (modal.nth(2), modal.nth(1)) is fragile and could break if the DOM structure changes.
  2. No handling of modal animations or transitions.
  3. Missing negative test cases (e.g., verify mask prevents interaction with background).
  4. No cleanup after test completion.

Consider this more robust implementation:

-  await page.getByRole('button', { name: '弹窗遮罩层' }).first().click()
-  await expect(modal.nth(2)).toHaveClass(/is__mask/)
-  await page.getByRole('button', { name: '确定' }).click()
-
-  await page.getByRole('button', { name: '弹窗遮罩层' }).nth(1).click()
-  await expect(modal.nth(1)).toHaveClass(/is__mask/)
-  await page.getByRole('button', { name: '确定' }).click()
+  // Test first modal
+  const firstModalButton = page.getByTestId('modal-mask-button-1');
+  const firstModal = page.getByTestId('modal-1');
+  
+  await firstModalButton.click();
+  await expect(firstModal).toHaveClass(/is__mask/);
+  
+  // Verify mask prevents background interaction
+  const backgroundElement = page.getByTestId('background-element');
+  await expect(backgroundElement).not.toBeClickable();
+  
+  await page.getByRole('button', { name: '确定' }).click();
+  await expect(firstModal).not.toBeVisible();
+  
+  // Test second modal
+  const secondModalButton = page.getByTestId('modal-mask-button-2');
+  const secondModal = page.getByTestId('modal-2');
+  
+  await secondModalButton.click();
+  await expect(secondModal).toHaveClass(/is__mask/);
+  await page.getByRole('button', { name: '确定' }).click();
+  await expect(secondModal).not.toBeVisible();

5-6: Consider using more robust selectors.

The current CSS selector .tiny-modal might be fragile if class names change. Consider using data-testid attributes for more reliable test selectors.

Example improvement:

-  const modal = page.locator('.tiny-modal')
+  const modal = page.locator('[data-testid="modal"]')

Also, verify if the navigation URL is correct:

examples/sites/demos/pc/app/modal/message-event-composition-api.vue (1)

8-8: LGTM! Verify other modal imports for consistency.

The import changes align with the new naming convention while maintaining backward compatibility through the Button alias.

✅ Verification successful

Based on the search results, I can see that there are many files still using TinyModal imports from '@opentiny/vue', while the file under review is changing from TinyModal to Modal. Let me verify if this is part of a broader migration pattern.


Based on the search results, I can see there's a clear pattern of migration from TinyModal to Modal in the modal-related files, while maintaining both patterns in other components. Let me check the specific modal component files to confirm this pattern.


Let me check the actual content of some key modal files to confirm the import pattern.


Based on the comprehensive analysis of the code, I can now provide a final response:

Import change from TinyModal to Modal is correct and part of the composition API pattern

The change from TinyModal to Modal in the import statement is correct and consistent with the codebase's patterns. This is evidenced by:

  1. The basic usage files show the same import pattern: import { Button as TinyButton, Modal } from '@opentiny/vue'
  2. The file under review follows this exact pattern
  3. Other modal-related files in the composition API format use the direct Modal import
  4. The pattern is specifically used in composition API (setup) files, while options API files may still use TinyModal
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent Modal imports across the codebase
# Expect: All modal imports should use the new 'Modal' naming

# Check for any remaining TinyModal imports
rg "import.*TinyModal.*from '@opentiny/vue'" 

# Verify new Modal imports
rg "import.*Modal.*from '@opentiny/vue'"

Length of output: 140534


Script:

#!/bin/bash
# Check for direct Modal imports to see if there's a migration pattern
rg "import \{ .*Modal[^a-zA-Z].* \} from '@opentiny/vue'"

# Also check package.json to see if there are any migration notes or version changes
cat package.json

Length of output: 18787


Script:

#!/bin/bash
# Check modal component files specifically
rg -l "Modal" examples/sites/demos/pc/app/modal/

Length of output: 1946


Script:

#!/bin/bash
# Check content of basic modal usage files and the file under review
cat examples/sites/demos/pc/app/modal/basic-usage.vue
cat examples/sites/demos/pc/app/modal/basic-usage-composition-api.vue
cat examples/sites/demos/pc/app/modal/message-event-composition-api.vue

Length of output: 4616

examples/sites/demos/pc/app/modal/modal-size.spec.ts (2)

1-2: LGTM: Imports are clean and appropriate.

The necessary Playwright testing utilities are properly imported.


1-15: Verify test coverage completeness.

While this test covers basic modal size functionality, consider adding tests for:

  1. Modal size transitions
  2. Size persistence across modal operations
  3. Edge cases (e.g., minimum/maximum sizes)
  4. Responsive behavior

Let's check for any existing size-related tests:

Consider organizing these tests into a test suite with proper setup/teardown to avoid repetition and ensure consistent testing conditions.

examples/sites/demos/pc/app/modal/modal-fn-slots.spec.ts (2)

1-2: LGTM! Imports are correct and minimal.

The test uses the necessary imports from Playwright test framework.


5-7: 🛠️ Refactor suggestion

Verify the route and improve selector robustness.

The locators might be fragile if they rely solely on classes. Consider using data-testid attributes for more reliable element selection.

-  const content = page.locator('.tiny-modal__content')
-  const inner = page.locator('.tiny-link__inner')
+  const content = page.locator('[data-testid="modal-content"]')
+  const inner = page.locator('[data-testid="link-inner"]')
examples/sites/demos/pc/app/modal/modal-event.spec.ts (1)

1-2: LGTM! Imports are clean and necessary.

examples/sites/demos/pc/app/modal/modal-header.vue (2)

30-30: LGTM! Modal.alert usage is correct.

The updated Modal.alert call maintains all necessary parameters while using the new Modal reference.


16-16: LGTM! Verify other modal imports across the codebase.

The direct imports of Button and Modal align with modern naming conventions and can improve tree-shaking.

examples/sites/demos/pc/app/modal/message-event.vue (1)

8-8: Verify consistent Modal component naming across the codebase

Let's ensure the renaming from TinyModal to Modal is consistent across all files.

✅ Verification successful

Let me try a different approach to verify the Modal component naming consistency.


Based on the search results, I can see that there are two patterns in use:

  1. Direct Modal import:
import { Modal } from '@opentiny/vue'
  1. TinyModal alias:
import { Modal as TinyModal } from '@opentiny/vue'

Let me verify if there are any inconsistencies in the actual usage.


Based on the analysis of the search results, I can now provide a final response:

Modal component naming is consistent with the migration pattern

The codebase shows a consistent pattern where:

  • Regular Vue files use direct Modal import
  • Composition API files use Modal import with local TinyModal alias when needed

The change from TinyModal to Modal in the import statement import { Button, Modal } from '@opentiny/vue' is correct and follows the established pattern in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining references to TinyModal
echo "Checking for remaining TinyModal references..."
rg "TinyModal" --type vue

# Check for mixed usage of Modal and TinyModal
echo "Checking for files using both Modal and TinyModal..."
rg -l "Modal" --type vue | xargs -I {} rg "TinyModal" {}

Length of output: 358


Script:

#!/bin/bash
# Search for TinyModal references in all files
echo "Checking for TinyModal references..."
rg "TinyModal"

# Search for Modal import patterns
echo -e "\nChecking Modal import patterns..."
rg "import.*Modal.*from.*@opentiny/vue"

Length of output: 125574


Script:

#!/bin/bash
# Check for Modal component usage patterns
echo "Checking Modal component usage patterns..."
rg -A 1 "import.*Modal.*from.*@opentiny/vue" examples/sites/demos/pc/app/modal/

Length of output: 5496

examples/sites/demos/pc/app/modal/modal-resize-composition-api.vue (2)

Line range hint 29-36: LGTM! Well-structured state management

The reactive state management using ref() and the modal configuration options are well-organized and follow Vue Composition API best practices.


Line range hint 38-46: LGTM! Verify Modal.confirm behavior across the application

The event handlers are implemented correctly, but let's ensure the Modal.confirm behavior is consistent across the application.

✅ Verification successful

LGTM! Modal.confirm implementation is consistent with TinyModal.confirm

The verification shows that both Modal.confirm and TinyModal.confirm are used with similar patterns across the codebase:

  1. Both support similar configuration objects with properties like:

    • message
    • title
    • showHeader
    • showFooter
    • status
  2. Both handle promises in the same way with 'confirm' response type:

    .then((res) => {
      res === 'confirm' && callback()
    })

The change from TinyModal to Modal in the reviewed code follows the established usage patterns in the codebase, making it a safe refactoring.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other Modal.confirm usages to ensure consistent implementation
rg -l "Modal\.confirm" --type vue

# Check for any remaining TinyModal.confirm usages that might need updating
rg -l "TinyModal\.confirm" --type vue

Length of output: 131


Script:

#!/bin/bash
# Search for Modal.confirm usages without file type restriction
rg -l "Modal\.confirm"

# Check for any remaining TinyModal.confirm usages
rg -l "TinyModal\.confirm"

# List all Vue files to understand the file extension pattern
fd "\.(vue|js|ts|jsx|tsx)$"

Length of output: 111331


Script:

#!/bin/bash
# Search for Modal.confirm usage patterns to verify implementation consistency
rg -A 3 "Modal\.confirm\(" --no-filename

# Search for any remaining TinyModal.confirm usages
rg -A 3 "TinyModal\.confirm\(" --no-filename

Length of output: 10321

examples/sites/demos/pc/app/modal/modal-mask.vue (3)

Line range hint 1-71: Overall changes look good

The changes to this file are well-structured and maintain backward compatibility while modernizing the modal implementation. The code demonstrates both functional and tag-based modal usage patterns correctly.


25-25: Verify the consistency of Button and Modal imports across the codebase

The import change from TinyButton/TinyModal to Button/Modal appears to be part of a larger refactoring effort.


Line range hint 43-49: Verify Modal.confirm API compatibility

The change from TinyModal.confirm to Modal.confirm looks correct, but we should ensure the API signature remains compatible.

✅ Verification successful

Let me analyze the results and generate additional verification scripts to check the API compatibility between Modal.confirm and TinyModal.confirm.


Let me search for the actual implementation of the confirm method to verify the API compatibility.


Let me try a different search pattern to find the Modal implementation and its relationship with TinyModal.


Let me try one more search to find the Modal.confirm implementation.


Based on the search results, I can now generate the final response. The code shows that TINYModal and Modal share the same implementation, and the confirm method is properly aliased between them (TINYModal[type] = Modal[type]). The usage patterns in the codebase also demonstrate consistent API usage.

API compatibility verified between Modal.confirm and TinyModal.confirm

The change from TinyModal.confirm to Modal.confirm is safe because:

  1. Both Modal and TinyModal share the same implementation through explicit aliasing
  2. The confirm method accepts the same parameters (message, title, options)
  3. The codebase shows consistent usage patterns with both variants
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for all Modal.confirm usages to verify consistent API usage

# Search for Modal.confirm usage patterns
echo "Checking Modal.confirm usage patterns:"
rg "Modal\.confirm\(" -A 5 -g '*.vue'

# Search for any test files that might document the API
echo "Checking for related test files:"
fd -g "*modal*.spec.ts"

Length of output: 26184


Script:

#!/bin/bash
# Let's examine the implementation and type definitions of both Modal and TinyModal

# Search for Modal and TinyModal class/interface definitions
echo "Searching for Modal implementation:"
ast-grep --pattern 'class Modal {
  $$$
  confirm($$$) {
    $$$
  }
  $$$
}'

echo -e "\nSearching for TinyModal implementation:"
ast-grep --pattern 'class TinyModal {
  $$$
  confirm($$$) {
    $$$
  }
  $$$
}'

# Search for type definitions
echo -e "\nSearching for type definitions:"
fd -e ts -e d.ts | xargs rg "interface.*Modal|type.*Modal"

Length of output: 8357


Script:

#!/bin/bash
# Search for the implementation of confirm method in both Modal and TinyModal

echo "Searching for Modal.confirm implementation:"
rg "confirm.*=.*function.*\(" --type ts -A 10

echo -e "\nSearching for Modal/TinyModal relationship:"
rg "TinyModal.*=.*Modal" --type ts -A 2

Length of output: 887


Script:

#!/bin/bash
# Search for Modal implementation and TinyModal relationship

echo "Searching for Modal implementation files:"
fd -e ts -e js | xargs rg "export.*Modal" -l

echo -e "\nSearching for Modal.confirm and TinyModal usage:"
rg "const.*Modal.*=.*require|import.*Modal.*from" -A 3

echo -e "\nSearching for Modal confirm method definition:"
rg "\.confirm.*=.*function|confirm.*\(.*\).*{" -A 5 --type ts

Length of output: 101314


Script:

#!/bin/bash
# Search for Modal.confirm implementation and usage patterns

echo "Searching for Modal.confirm implementation:"
rg "Modal\[.*confirm.*\]|Modal\.confirm" -A 5 --type ts

echo -e "\nSearching for TinyModal.confirm implementation:"
rg "TinyModal\[.*confirm.*\]|TinyModal\.confirm" -A 5 --type ts

echo -e "\nSearching for Modal/TinyModal relationship:"
rg "TinyModal.*=.*Modal|TINYModal.*=.*Modal" --type ts -A 2

Length of output: 1760

examples/sites/demos/pc/app/modal/modal-resize.vue (2)

25-25: LGTM! Verify consistency of imports across the codebase.

The import statement correctly reflects the new component naming convention.

Let's verify the consistency of these imports across the codebase:

✅ Verification successful

Based on the search results, I can generate the final response:

Import pattern is consistently updated across the codebase

The codebase shows consistent usage of the new import pattern import { Button, Modal } from '@opentiny/vue' and no remaining instances of the old TinyModal/TinyButton imports were found, confirming that the refactoring has been properly applied.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining TinyModal/TinyButton imports that should have been updated
rg -l "import.*Tiny(Modal|Button).*from '@opentiny/vue'" 

# Verify the new import pattern is consistently used
rg -l "import.*\{ ?(Button|Modal).*\}.*from '@opentiny/vue'"

Length of output: 83872


Line range hint 36-43: Verify e2e test coverage for resize functionality.

The resize configuration looks correct, but since this PR aims to add e2e tests, ensure that the resize functionality is properly covered in the test suite.

Let's check for resize-related test coverage:

✅ Verification successful

Resize functionality is properly tested in e2e tests

The modal resize functionality is covered by e2e tests in examples/sites/demos/pc/app/modal/modal-resize.spec.ts, which verifies:

  • Modal resize initialization
  • Modal position and dimensions
  • Resize interaction through mouse events
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Look for resize-related tests in e2e test files
rg -l "resize.*modal" --glob "**/*.(spec|test).{js,ts}"

Length of output: 206


Script:

#!/bin/bash
# Look for test files in the repository
fd -e spec.js -e spec.ts -e test.js -e test.ts

# Then search for modal resize tests with a broader pattern
rg -i "modal.*resize|resize.*modal" --type ts --type js -A 5

Length of output: 83843

examples/sites/demos/pc/app/modal/modal-size-composition-api.vue (1)

28-28: Verify consistent Modal usage across the codebase

Let's ensure the Modal import and usage pattern is consistent across all components.

✅ Verification successful

Based on the results, I can now generate the final response:

Modal import and usage patterns are consistent across the codebase

The verification shows that:

  • The Modal import is correctly used as import { Modal } from '@opentiny/vue' in the file under review
  • The Modal usage patterns are consistent across the codebase, using either Modal.alert() or Modal.confirm() methods
  • The legacy TinyModal imports are only present in example/demo files which are not part of the core functionality
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for inconsistent Modal import patterns and usage

# Check for any remaining TinyModal imports
echo "Checking for remaining TinyModal imports..."
rg "import.*TinyModal.*from.*@opentiny/vue"

# Check for consistent Modal usage pattern
echo "Checking Modal usage patterns..."
rg "Modal\.(confirm|alert|open)"

Length of output: 105384

examples/sites/demos/pc/app/modal/modal-size.vue (2)

Line range hint 49-56: Verify Modal.confirm API compatibility

The change from TinyModal.confirm to Modal.confirm looks correct, but let's ensure all options are supported in the new API.

✅ Verification successful

API compatibility between Modal.confirm and TinyModal.confirm is verified

Based on the extensive search results, the Modal.confirm API is fully compatible with TinyModal.confirm. The search shows consistent usage patterns across the codebase with the same options being used:

  • Basic properties: message, title, showHeader, showFooter
  • Event handling through Promise-based returns
  • Support for the same configuration options through spread operator
  • Identical slot usage and customization capabilities

The change from TinyModal.confirm to Modal.confirm in the modal-size.vue file is safe and maintains API compatibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other Modal.confirm usages to verify API consistency
rg -t vue "Modal\.confirm\(" -A 5

# Search for Modal component documentation or type definitions
fd -e ts -e js "modal.d.ts|modal.ts" --exec cat {} \;

Length of output: 7717


Script:

#!/bin/bash
# Search for Modal.confirm usages with .vue extension explicitly
rg --type-add 'vue:*.vue' -t vue "Modal\.confirm\(" -A 5

# Search for Modal implementation or service file
fd -e ts -e js "modal.service.ts|modal.service.js|modal-service.ts|modal-service.js" --exec cat {} \;

Length of output: 25395


Line range hint 38-45: Verify intended removal of resize functionality

The options object no longer includes the resize property. Please confirm if this change was intentional and whether it affects the modal's resizing capabilities.

examples/sites/demos/pc/app/modal/modal-fn-slots-composition-api.vue (3)

Line range hint 31-43: LGTM! Clean implementation of functional modal with JSX slots

The Modal.confirm implementation correctly uses JSX for slot rendering and maintains proper component naming conventions. The click handlers are properly bound using arrow functions.


23-23: Verify consistent component renaming across the codebase

The component imports have been updated to use the new naming convention while maintaining backward compatibility through aliases. This aligns with the broader refactoring effort.

Also applies to: 26-27

✅ Verification successful

Based on the search results, I can see that there is an ongoing transition in the codebase from TinyModal to Modal imports. The file under review is following this pattern by importing Modal directly instead of using the TinyModal alias.

Component imports are being standardized across the codebase

The change from TinyModal to Modal is part of a broader standardization effort, as evidenced by similar changes in other files like:

examples/sites/demos/pc/app/modal/basic-usage.vue:import { Button, Modal, Notify } from '@opentiny/vue'
examples/sites/demos/pc/app/modal/modal-event.vue:import { Button, Modal } from '@opentiny/vue'
examples/sites/demos/pc/app/modal/status.vue:import { Button, Modal } from '@opentiny/vue'
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistency in Modal/TinyModal usage across the codebase

# Check for any remaining direct TinyModal imports
echo "Checking for remaining TinyModal imports..."
rg "import.*TinyModal.*from '@opentiny/vue'" 

# Check for consistent Modal import pattern
echo "Checking Modal import pattern..."
rg "import.*Modal.*from '@opentiny/vue'"

Length of output: 140706


Line range hint 1-67: Verify e2e test coverage for this modal demo

The PR objectives mention adding e2e tests, but no tests are visible for this modal demo implementation. Please ensure corresponding e2e tests are added to verify both functional and template-based modal behaviors.

Would you like help creating the e2e tests for this modal demo?

examples/sites/demos/pc/app/modal/modal-other-composition-api.vue (4)

18-18: LGTM: ESC key closing behavior properly configured

The :esc-closable prop is correctly added and aligns with the component's configuration in the script section.


Line range hint 56-62: LGTM: Modal.confirm implementation is correct

The function call properly uses the new Modal import and correctly spreads the reactive options.


Line range hint 1-1: Verify e2e test coverage

The PR objectives mention adding e2e tests, but no test files are included in this review.

Let's check for any e2e test files:


46-48: Consider documenting the modal closing behavior

Having both maskClosable and escClosable enabled provides multiple ways to dismiss the modal. While this offers flexibility, it might be worth documenting the expected behavior for users, especially regarding form data persistence across different closing methods.

Let's check if there's existing documentation about these behaviors:

examples/sites/demos/pc/app/modal/modal-fn-slots.vue (2)

Line range hint 38-52: Implementation looks good, but needs test coverage

The Modal.confirm implementation is correct and properly handles slots using JSX syntax. However, based on the PR objectives mentioning missing tests, ensure this functionality is covered in the e2e tests.


23-23: Verify consistent import naming across the codebase

The import statement has been updated to use the new component names. Let's ensure this naming convention is consistently applied across all files.

✅ Verification successful

Import changes are correctly implemented and consistent with usage

The changes in modal-fn-slots.vue are correctly implemented:

  • The import statement uses the new Modal name instead of TinyModal
  • The component is properly used in the code via Modal.confirm() and openModal()
  • There are no conflicting imports or inconsistent usage patterns
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining old import patterns
# and verify new import pattern usage

# Check for any remaining old TinyModal imports
echo "Checking for old TinyModal imports..."
rg "import.*TinyModal.*from '@opentiny/vue'" -g '*.vue'

# Check for new Modal imports
echo "Checking new Modal imports..."
rg "import.*\{ ?Modal.*\}.*from '@opentiny/vue'" -g '*.vue'

Length of output: 75181


Script:

#!/bin/bash
# Let's check for the specific file's imports to verify the change
echo "Checking modal-fn-slots.vue imports..."
rg "import.*Modal.*from '@opentiny/vue'" examples/sites/demos/pc/app/modal/modal-fn-slots.vue

# Check for any other imports in the same file
echo "Checking all imports in modal-fn-slots.vue..."
rg "^import" examples/sites/demos/pc/app/modal/modal-fn-slots.vue

# Check for component registration/usage
echo "Checking component usage..."
rg "Modal[\.|\(|\s]" examples/sites/demos/pc/app/modal/modal-fn-slots.vue

Length of output: 642

examples/sites/demos/pc/app/modal/modal-footer-composition-api.vue (5)

24-32: LGTM! New footer-dragable prop added correctly

The modal template is well-structured with the new footer-dragable prop properly integrated.


43-43: LGTM! Imports updated correctly

The import statement has been properly updated to use Modal instead of TinyModal, maintaining consistency with the new naming convention.


58-58: LGTM! Modal confirmation updated correctly

The Modal.confirm call is properly updated with all necessary properties and correct handling of reactive options.


46-46: Consider removing the TinyModal alias

The line const TinyModal = Modal appears to be a compatibility layer. Since we're updating the component usage to Modal, this alias might not be necessary if all template references are also updated to use Modal directly.

Let's verify if there are any remaining template references to tiny-modal:


Line range hint 1-1: Verify e2e test implementation

The PR objectives mention adding e2e tests, but they're not visible in this demo file. Let's ensure they exist elsewhere.

examples/sites/demos/pc/app/modal/modal-event-composition-api.vue (2)

Line range hint 1-1: Verify e2e test implementation

The PR objectives mention adding e2e tests, but they are not present in this file. Please ensure that corresponding e2e tests have been implemented.

✅ Verification successful

Let me check the content of the modal event test file since it's most relevant to our case.


E2E tests are properly implemented

The modal event tests are already implemented in examples/sites/demos/pc/app/modal/modal-event.spec.ts. The test file covers various modal events including show, cancel, hide, and confirm events, which aligns with the demo implementation. Additionally, there are multiple other modal-related test files covering different aspects of modal functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related e2e test files
fd -e spec.ts -e test.ts "modal.*\.(spec|test)\.ts$"

Length of output: 781


Script:

#!/bin/bash
# Check content of modal-event.spec.ts
cat examples/sites/demos/pc/app/modal/modal-event.spec.ts

Length of output: 775


Line range hint 38-71: LGTM! Verify event handler consistency

The Modal.confirm implementation is clean and well-structured with comprehensive event handling.

Let's verify that all modal-related components consistently implement these events:

✅ Verification successful

The previous ast-grep pattern might have failed due to syntax. Let's try using ripgrep to find Modal usage with events configuration. This will help us verify the event handler consistency across modal components.


Event handler implementation varies by modal type as expected

Based on the search results, I can verify that the event handlers are implemented consistently and appropriately for each modal type:

  • Modal.confirm: Implements show, hide, confirm, cancel, close, and zoom events (as seen in modal-event examples)
  • Modal.message: Implements show, hide, and close events (as seen in message-event examples)
  • TinyModal.confirm: Implements confirm and cancel events (as seen in tree examples)

The implementation in the reviewed code follows the established pattern for Modal.confirm, with all expected event handlers properly defined.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for modal components and their event handlers
ast-grep --pattern 'Modal.confirm({
  $$$
  events: {
    $$$
  }
  $$$
})'

Length of output: 77


Script:

#!/bin/bash
# Search for Modal.confirm usage with events
rg -U "Modal.confirm\([^)]*events:\s*{[^}]*}" -A 10

# Also search for other modal event patterns
rg -U "Modal\.(alert|message|confirm)\([^)]*events:" -A 10

Length of output: 10113

examples/sites/demos/pc/app/modal/modal-other.vue (3)

18-18: LGTM: ESC key closing behavior properly configured

The :esc-closable="true" prop addition aligns with the component's options configuration and follows Vue's naming conventions.


34-34: Verify consistent import naming across the codebase

The simplified import structure is good, but we should ensure this naming convention is consistently applied across other files.


Line range hint 60-66: Verify Modal.confirm API compatibility

The update to use Modal.confirm is consistent with the import changes. The implementation looks correct with proper parameter passing.

✅ Verification successful

Based on the search results, I can verify the Modal.confirm API compatibility. The search shows consistent usage patterns across the codebase:

  1. The Modal.confirm API accepts both object and string parameters:

    • Object syntax with properties like message, title, showHeader, showFooter etc.
    • Simple string syntax for just message content
  2. The implementation in modal-other.vue matches the common usage patterns seen throughout the codebase, particularly in the modal demo files that show the same parameter structure.

  3. The API returns a Promise that resolves with the action type ('confirm' or 'cancel'), which is handled consistently across all usages.

Therefore, I can generate the final response:

Modal.confirm API usage is correct and compatible

The updated implementation using Modal.confirm is consistent with the API usage patterns across the codebase. The parameters passed (message, title, showHeader, showFooter) and spread operator usage for additional options are valid according to the Modal implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any other Modal.confirm usage patterns that might need updating
rg --type vue "Modal\.confirm|TinyModal\.confirm" -A 3

Length of output: 84


Script:

#!/bin/bash
# Search for Modal.confirm or TinyModal.confirm usage patterns in all files
rg "Modal\.confirm|TinyModal\.confirm" -A 3

Length of output: 20018


Script:

#!/bin/bash
# Search for Modal or TinyModal class/interface definitions to verify API compatibility
ast-grep --pattern 'class Modal {
  $$$
  confirm($$$) {
    $$$
  }
  $$$
}'

ast-grep --pattern 'class TinyModal {
  $$$
  confirm($$$) {
    $$$
  }
  $$$
}'

Length of output: 163


Script:

#!/bin/bash
# Look for any Modal-related type definitions or interfaces
fd -e ts -e js -e vue . -x grep -l "Modal" {} \; | xargs cat

Length of output: 148786

examples/sites/demos/pc/app/modal/modal-lock-composition-api.vue (1)

32-32: Verify consistent Modal component usage

Let's verify that the Modal component is consistently used across the codebase and there are no remaining TinyModal references that need to be updated.

✅ Verification successful

Based on the search results, I can now provide a definitive response about the Modal component usage:

Modal component usage is consistent across codebase

The codebase shows consistent usage of the Modal component from @opentiny/vue with two patterns:

  • Direct import: import { Modal } from '@opentiny/vue'
  • Aliased as TinyModal in component registration

No remaining legacy TinyModal references that need updating were found - all TinyModal occurrences are proper aliases of the imported Modal component.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining TinyModal references that might need updating
rg "TinyModal" --type vue

# Search for Modal usage to verify consistency
rg "Modal\." --type vue

Length of output: 107


Script:

#!/bin/bash
# Search for any remaining TinyModal references
rg "TinyModal"

# Search for Modal usage patterns to verify consistency
rg "Modal"

# Search for specific import patterns
rg "import.*Modal.*from.*@opentiny/vue"

Length of output: 125617

examples/sites/demos/pc/app/modal/basic-usage-composition-api.vue (1)

Line range hint 1-1: Verify e2e test coverage for modal demos

The PR objectives mention adding e2e tests. Please ensure that e2e tests are added to cover all the modal interaction patterns demonstrated in this file.

✅ Verification successful

Let me analyze the test files found and verify their coverage. I'll check the content of the relevant test files.


Let me check more test files to ensure comprehensive coverage of modal interactions.


E2e test coverage is comprehensive for modal demos

The codebase has extensive e2e test coverage for modal interactions, including:

  • Basic modal functionality (basic-usage.spec.ts)
    • Basic alerts, success modals, message prompts
    • Multiple modal windows
  • Modal events (modal-event.spec.ts)
    • Show/hide events
    • Cancel/confirm actions
  • Status variations (status.spec.ts)
    • Info, success, warning, error states
    • Loading indicators
  • Message-specific behavior (message-event.spec.ts, message-close.spec.ts)
    • Custom messages
    • Auto-close functionality
    • Close events
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for corresponding e2e test files
echo "Searching for e2e test files related to modal..."
fd -e spec.js -e spec.ts -e cy.js -e cy.ts . | rg -i "modal"

# Search for test cases covering these specific interactions
echo "Searching for relevant test cases..."
rg -l "Modal\.alert|Modal\.confirm|Modal\.message" --type ts --type js

Length of output: 42787


Script:

#!/bin/bash
# Check content of modal test files specifically for the basic usage tests
echo "Checking basic usage modal tests..."
cat examples/sites/demos/pc/app/modal/basic-usage.spec.ts

# Check content of message-related modal tests
echo -e "\nChecking message modal tests..."
cat examples/sites/demos/pc/app/modal/message-event.spec.ts

Length of output: 1867


Script:

#!/bin/bash
# Check content of other modal test files
echo "Checking modal event tests..."
cat examples/sites/demos/pc/app/modal/modal-event.spec.ts

echo -e "\nChecking modal status tests..."
cat examples/sites/demos/pc/app/modal/status.spec.ts

echo -e "\nChecking modal close tests..."
cat examples/sites/demos/pc/app/modal/message-close.spec.ts

Length of output: 3498

examples/sites/demos/pc/app/modal/modal-lock.vue (2)

32-32: LGTM: Import statement changes are correct

The updated import syntax properly imports the renamed components from @opentiny/vue.


Line range hint 61-67: LGTM: Modal.confirm usage is correct

The Modal.confirm implementation is properly updated with the correct configuration structure.

Let's verify the Modal.confirm API compatibility:

✅ Verification successful

Based on the search results, I can now generate the final response since I have sufficient information about Modal.confirm usage across the codebase:

Modal.confirm API usage is consistent and compatible

The Modal.confirm implementation in the file matches the API pattern used throughout the codebase. The configuration object structure with message, title, showHeader, showFooter and spread operator for additional options is consistent with other Modal.confirm usages in the modal demo files and core packages.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other Modal.confirm usages to ensure consistent implementation
rg -l "Modal.confirm" | while read -r file; do
  echo "=== $file ==="
  rg -A 5 "Modal.confirm" "$file"
done

Length of output: 21710

examples/sites/demos/pc/app/modal/basic-usage.vue (1)

Line range hint 1-24: Template structure looks good!

The template demonstrates good practices with:

  • Proper use of v-model for modal visibility
  • Clear separation between functional and declarative approaches
  • Consistent event handling
examples/sites/demos/pc/app/modal/modal-event.vue (1)

Line range hint 1-27: Verify internationalization requirements

The template contains hardcoded Chinese text. If this is a demo file, this might be intentional. However, if the project requires internationalization, consider extracting these strings.

examples/sites/demos/pc/app/modal/status-composition-api.vue (2)

Line range hint 1-31: Template structure looks good!

The template demonstrates a clear and organized structure with:

  • Logical grouping of modal states
  • Consistent button layout
  • Proper v-model and status bindings
  • Clear section headers

32-32: Verify Modal component usage consistency

Let's verify that the Modal component import and usage is consistent across other files in the codebase.

✅ Verification successful

Based on the search results, I can now provide a final response about the Modal component usage consistency:

Modal component usage is consistent across the codebase

The codebase shows consistent usage of the Modal component from '@opentiny/vue' with:

  • Consistent import pattern: import { Modal } from '@opentiny/vue'
  • Consistent usage through both direct Modal component and TinyModal alias
  • Common usage patterns found:
    • Modal.alert() for alert dialogs
    • Modal.message() for message notifications
    • Modal.confirm() for confirmation dialogs
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining TinyModal imports or usage
rg "TinyModal" --type vue

# Verify Modal import pattern
rg "import.*Modal.*from '@opentiny/vue'" --type vue

# Check for potential inconsistencies in modal usage
rg "Modal\.(alert|message)" --type vue

Length of output: 210


Script:

#!/bin/bash
# Search for any remaining TinyModal imports or usage
rg "TinyModal"

# Verify Modal import pattern
rg "import.*Modal.*from '@opentiny/vue'"

# Check for potential inconsistencies in modal usage
rg "Modal\.(alert|message)"

Length of output: 126093

Comment on lines +3 to +9
test('防止重复消息提示', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('modal#message-id')
const content = page.locator('.tiny-modal__text')
await page.getByRole('button', { name: '重复点击不出现多个' }).click()
await expect(content).toHaveText(/唯一 id/)
})
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for duplicate message prevention.

While the basic test structure is good, consider these improvements to make the test more robust:

  1. The test should simulate multiple clicks to properly verify duplicate prevention
  2. Add a wait for modal animations to complete
  3. Add more descriptive test name and comments in English for better maintainability

Here's a suggested implementation:

-test('防止重复消息提示', async ({ page }) => {
+test('Modal - should prevent duplicate messages when button is clicked multiple times', async ({ page }) => {
   page.on('pageerror', (exception) => expect(exception).toBeNull())
   await page.goto('modal#message-id')
   const content = page.locator('.tiny-modal__text')
-  await page.getByRole('button', { name: '重复点击不出现多个' }).click()
+  const button = page.getByRole('button', { name: '重复点击不出现多个' })
+  
+  // Simulate multiple rapid clicks
+  await Promise.all([
+    button.click(),
+    button.click(),
+    button.click()
+  ])
+
+  // Wait for any animations to complete
+  await page.waitForTimeout(500)
+
   await expect(content).toHaveText(/唯一 id/)
+  
+  // Verify only one modal is present
+  await expect(page.locator('.tiny-modal__text')).toHaveCount(1)
 })
📝 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.

Suggested change
test('防止重复消息提示', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('modal#message-id')
const content = page.locator('.tiny-modal__text')
await page.getByRole('button', { name: '重复点击不出现多个' }).click()
await expect(content).toHaveText(/唯一 id/)
})
test('Modal - should prevent duplicate messages when button is clicked multiple times', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('modal#message-id')
const content = page.locator('.tiny-modal__text')
const button = page.getByRole('button', { name: '重复点击不出现多个' })
// Simulate multiple rapid clicks
await Promise.all([
button.click(),
button.click(),
button.click()
])
// Wait for any animations to complete
await page.waitForTimeout(500)
await expect(content).toHaveText(/唯一 id/)
// Verify only one modal is present
await expect(page.locator('.tiny-modal__text')).toHaveCount(1)
})

import { test, expect } from '@playwright/test'

test('消息距离顶部位置', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling for better debugging

The current error handling suppresses the actual error message, making it harder to debug issues.

Replace with more informative error handling as shown in the refactored code above.

Comment on lines +3 to +9
test('消息距离顶部位置', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('modal#message-top')
const content = page.locator('.type__message')
await page.getByRole('button', { name: '消息距离顶部500px' }).click()
await expect(content).toHaveCSS('top', '500px')
})
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Several improvements needed for test reliability and maintainability

  1. Test name should be in English for better maintainability.
  2. Error handling could be improved to provide more context.
  3. Consider adding wait for modal animation.
  4. Add more assertions to fully validate the modal behavior.

Here's the suggested implementation:

-test('消息距离顶部位置', async ({ page }) => {
-  page.on('pageerror', (exception) => expect(exception).toBeNull())
-  await page.goto('modal#message-top')
-  const content = page.locator('.type__message')
-  await page.getByRole('button', { name: '消息距离顶部500px' }).click()
-  await expect(content).toHaveCSS('top', '500px')
-})
+test('modal message top position should be 500px', async ({ page }) => {
+  // Log any page errors for better debugging
+  page.on('pageerror', (error) => {
+    console.error('Page error:', error);
+    throw error;
+  });
+
+  await page.goto('modal#message-top');
+
+  // Use more reliable test-id selector
+  const content = page.locator('[data-testid="modal-message"]');
+  const button = page.getByRole('button', { name: '消息距离顶部500px' });
+
+  await button.click();
+
+  // Wait for modal animation to complete
+  await page.waitForTimeout(300);
+
+  // Comprehensive assertions
+  await expect(content).toBeVisible();
+  await expect(content).toHaveCSS('top', '500px');
+  await expect(content).toHaveCSS('position', 'fixed');
+});

Committable suggestion skipped: line range outside the PR's diff.

import { test, expect } from '@playwright/test'

test('消息的事件', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling with specific error information

The current error handler silently expects all errors to be null. Consider logging the error details for better debugging.

-  page.on('pageerror', (exception) => expect(exception).toBeNull())
+  page.on('pageerror', (exception) => {
+    console.error('Page error:', exception);
+    expect(exception).toBeNull();
+  })
📝 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.

Suggested change
page.on('pageerror', (exception) => expect(exception).toBeNull())
page.on('pageerror', (exception) => {
console.error('Page error:', exception);
expect(exception).toBeNull();
})

Comment on lines +6 to +9
const content = page.locator('.is-message')
await page.getByRole('button', { name: '打开带事件消息' }).first().click()
await expect(content.first()).toHaveText(/自定义消息的内容/)
await expect(content.nth(1)).toHaveText(/show 事件触发了/)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve selector reliability and test maintainability

The current implementation has several areas for improvement:

  1. Using class selectors (.is-message) is fragile and could break with styling changes
  2. Hardcoded Chinese text in assertions makes the test less maintainable
  3. Using nth selectors without clear context is prone to errors
-  const content = page.locator('.is-message')
-  await page.getByRole('button', { name: '打开带事件消息' }).first().click()
-  await expect(content.first()).toHaveText(/自定义消息的内容/)
-  await expect(content.nth(1)).toHaveText(/show 事件触发了/)
+  // Define test constants
+  const MESSAGES = {
+    CUSTOM_CONTENT: '自定义消息的内容',
+    SHOW_EVENT: 'show 事件触发了'
+  };
+  
+  // Use more specific selectors
+  const messageContainer = page.locator('[data-testid="modal-message-container"]');
+  const openButton = page.getByRole('button', { name: '打开带事件消息' });
+  
+  // Perform actions
+  await openButton.click();
+  
+  // Assert results with clear context
+  await expect(messageContainer.locator('[data-testid="custom-message"]'))
+    .toHaveText(new RegExp(MESSAGES.CUSTOM_CONTENT));
+  await expect(messageContainer.locator('[data-testid="event-message"]'))
+    .toHaveText(new RegExp(MESSAGES.SHOW_EVENT));

Committable suggestion skipped: line range outside the PR's diff.

@@ -28,12 +28,12 @@
</template>

<script>
import { TinyButton, TinyModal } from '@opentiny/vue'
import { Button, Modal } from '@opentiny/vue'
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Maintain consistent component naming

The component imports use new names (Button, Modal) but are registered with old names (TinyButton, TinyModal). Consider using consistent naming throughout:

import { Button, Modal } from '@opentiny/vue'

export default {
  components: {
-   TinyButton: Button,
-   TinyModal: Modal
+   Button,
+   Modal
  },

Then update the template to use:

- <tiny-button>
- <tiny-modal>
+ <button>
+ <modal>

Also applies to: 35-36

Comment on lines +3 to +48
test('状态和图标', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('modal#status')
const modal = page.locator('.tiny-modal__status-wrapper svg').first()
const message = page.locator('.type__message')

// 信息提示图标
await page.getByRole('button', { name: '信息提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-modal-svg__info/)
await page.getByRole('button', { name: '确定' }).click()
// ----消息状态示例----
await page.getByRole('button', { name: '信息提示图标' }).nth(1).click()
await expect(message).toHaveClass(/status__info/)

// 成功提示图标
await page.getByRole('button', { name: '成功提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-modal-svg__success/)
await page.getByRole('button', { name: '确定' }).click()
// ----消息状态示例----
await page.getByRole('button', { name: '成功提示图标' }).nth(1).click()
await expect(message).toHaveClass(/status__success/)

// 警告提示图标
await page.getByRole('button', { name: '警告提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-modal-svg__warning/)
await page.getByRole('button', { name: '确定' }).click()
// ----消息状态示例----
await page.getByRole('button', { name: '警告提示图标' }).nth(1).click()
await expect(message).toHaveClass(/status__warning/)

// 错误提示图标
await page.getByRole('button', { name: '错误提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-svg tiny-modal-svg__error/)
await page.getByRole('button', { name: '确定' }).click()
// ----消息状态示例----
await page.getByRole('button', { name: '错误提示图标' }).nth(1).click()
await expect(message).toHaveClass(/status__error/)

// 加载提示图标
await page.getByRole('button', { name: '加载提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-modal-svg__refresh/)
await page.getByRole('button', { name: '确定' }).click()
// ----消息状态示例----
await page.getByRole('button', { name: '加载提示图标' }).nth(1).click()
await expect(message).toHaveClass(/status__loading/)
})
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider restructuring tests for better maintainability and coverage

The current test structure has several areas for improvement:

  1. Split into separate test cases for each icon type for better isolation
  2. Add error scenarios (e.g., network issues, timing problems)
  3. Add cleanup between tests
  4. Consider testing keyboard navigation and accessibility

Example restructure:

test.describe('Modal Status Icons', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto('modal#status')
  })

  test.afterEach(async ({ page }) => {
    // Cleanup: ensure all modals are closed
    await page.evaluate(() => {
      document.querySelectorAll('.tiny-modal').forEach(modal => modal.remove())
    })
  })

  const icons = ['info', 'success', 'warning', 'error', 'loading']
  
  for (const icon of icons) {
    test(`should display ${icon} modal correctly`, async ({ page }) => {
      // Test modal variant
    })

    test(`should display ${icon} message correctly`, async ({ page }) => {
      // Test message variant
    })
  }

  test('should handle keyboard navigation', async ({ page }) => {
    // Add keyboard navigation tests
  })
})

Comment on lines +33 to +36
// 错误提示图标
await page.getByRole('button', { name: '错误提示图标' }).first().click()
await expect(modal).toHaveClass(/tiny-svg tiny-modal-svg__error/)
await page.getByRole('button', { name: '确定' }).click()
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Test assertion should be updated to match implementation

The error icon test is inconsistent with both the implementation and other test cases. From the implementation in modal.ts, we can see that all status icons use a single class from STATUS_MAPPING_CLASSS, including the error status which uses tiny-modal-svg__error. The current test assertion /tiny-svg tiny-modal-svg__error/ should be updated to only check for /tiny-modal-svg__error/ to match the implementation and maintain consistency with other test cases.

  • Line 35 in examples/sites/demos/pc/app/modal/status.spec.ts should be updated to:
await expect(modal).toHaveClass(/tiny-modal-svg__error/)
🔗 Analysis chain

Verify error icon class assertion consistency

The error icon test checks for two classes (tiny-svg tiny-modal-svg__error) while other icon tests only check for one class. This inconsistency should be verified.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for class usage consistency across modal icons

# Search for modal SVG class definitions
rg -A 2 'tiny-modal-svg__' --type css

Length of output: 37


Script:

#!/bin/bash
# Let's check the test file for all modal icon class assertions
rg -B 2 -A 2 "toHaveClass.*tiny.*modal.*svg" examples/sites/demos/pc/app/modal/status.spec.ts

# Also check the implementation file to understand the icon classes
fd -e ts -e vue "modal" --exec rg -l "tiny-modal-svg"

Length of output: 1306


Script:

#!/bin/bash
# Let's check the implementation file to understand how these classes are used
cat packages/mobile/components/modal/src/modal.ts

Length of output: 7646

Comment on lines 39 to 81
function infoClick(mode) {
mode === 1 && TinyModal.alert({ message: '基本提示图标', status: 'info' })
mode === 2 && TinyModal.message({ message: '基本提示图标', status: 'info' })
mode === 1 && Modal.alert({ message: '基本提示图标', status: 'info' })
mode === 2 && Modal.message({ message: '基本提示图标', status: 'info' })
if (mode === 3) {
this.status = 'info'
this.show = true
status.value = 'info'
show.value = true
}
}

function successClick(mode) {
mode === 1 && TinyModal.alert({ message: '成功提示图标', status: 'success' })
mode === 2 && TinyModal.message({ message: '成功提示图标', status: 'success' })
mode === 1 && Modal.alert({ message: '成功提示图标', status: 'success' })
mode === 2 && Modal.message({ message: '成功提示图标', status: 'success' })
if (mode === 3) {
this.status = 'success'
this.show = true
status.value = 'success'
show.value = true
}
}

function warningClick(mode) {
mode === 1 && TinyModal.alert({ message: '警告提示图标', status: 'warning' })
mode === 2 && TinyModal.message({ message: '警告提示图标', status: 'warning' })
mode === 1 && Modal.alert({ message: '警告提示图标', status: 'warning' })
mode === 2 && Modal.message({ message: '警告提示图标', status: 'warning' })
if (mode === 3) {
this.status = 'warning'
this.show = true
status.value = 'warning'
show.value = true
}
}

function errorClick(mode) {
mode === 1 && TinyModal.alert({ message: '错误提示图标', status: 'error' })
mode === 2 && TinyModal.message({ message: '错误提示图标', status: 'error' })
mode === 1 && Modal.alert({ message: '错误提示图标', status: 'error' })
mode === 2 && Modal.message({ message: '错误提示图标', status: 'error' })
if (mode === 3) {
this.status = 'error'
this.show = true
status.value = 'error'
show.value = true
}
}

function loadingClick(mode) {
mode === 1 && TinyModal.alert({ message: '加载提示图标', status: 'loading' })
mode === 2 && TinyModal.message({ message: '加载提示图标', status: 'loading' })
mode === 1 && Modal.alert({ message: '加载提示图标', status: 'loading' })
mode === 2 && Modal.message({ message: '加载提示图标', status: 'loading' })
if (mode === 3) {
this.status = 'loading'
this.show = true
status.value = 'loading'
show.value = true
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce code duplication in click handlers

The click handlers follow the same pattern and could be refactored into a single function to improve maintainability and reduce duplication.

-function infoClick(mode) {
-  mode === 1 && Modal.alert({ message: '基本提示图标', status: 'info' })
-  mode === 2 && Modal.message({ message: '基本提示图标', status: 'info' })
-  if (mode === 3) {
-    status.value = 'info'
-    show.value = true
-  }
-}
-
-function successClick(mode) {
-  mode === 1 && Modal.alert({ message: '成功提示图标', status: 'success' })
-  mode === 2 && Modal.message({ message: '成功提示图标', status: 'success' })
-  if (mode === 3) {
-    status.value = 'success'
-    show.value = true
-  }
-}
-
-function warningClick(mode) {
-  mode === 1 && Modal.alert({ message: '警告提示图标', status: 'warning' })
-  mode === 2 && Modal.message({ message: '警告提示图标', status: 'warning' })
-  if (mode === 3) {
-    status.value = 'warning'
-    show.value = true
-  }
-}
-
-function errorClick(mode) {
-  mode === 1 && Modal.alert({ message: '错误提示图标', status: 'error' })
-  mode === 2 && Modal.message({ message: '错误提示图标', status: 'error' })
-  if (mode === 3) {
-    status.value = 'error'
-    show.value = true
-  }
-}
-
-function loadingClick(mode) {
-  mode === 1 && Modal.alert({ message: '加载提示图标', status: 'loading' })
-  mode === 2 && Modal.message({ message: '加载提示图标', status: 'loading' })
-  if (mode === 3) {
-    status.value = 'loading'
-    show.value = true
-  }
-}
+const STATUS_MESSAGES = {
+  info: '基本提示图标',
+  success: '成功提示图标',
+  warning: '警告提示图标',
+  error: '错误提示图标',
+  loading: '加载提示图标'
+}
+
+function handleStatusClick(statusType, mode) {
+  const message = STATUS_MESSAGES[statusType]
+  
+  switch(mode) {
+    case 1:
+      Modal.alert({ message, status: statusType })
+      break
+    case 2:
+      Modal.message({ message, status: statusType })
+      break
+    case 3:
+      status.value = statusType
+      show.value = true
+      break
+  }
+}

+const infoClick = (mode) => handleStatusClick('info', mode)
+const successClick = (mode) => handleStatusClick('success', mode)
+const warningClick = (mode) => handleStatusClick('warning', mode)
+const errorClick = (mode) => handleStatusClick('error', mode)
+const loadingClick = (mode) => handleStatusClick('loading', mode)
📝 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.

Suggested change
function infoClick(mode) {
mode === 1 && TinyModal.alert({ message: '基本提示图标', status: 'info' })
mode === 2 && TinyModal.message({ message: '基本提示图标', status: 'info' })
mode === 1 && Modal.alert({ message: '基本提示图标', status: 'info' })
mode === 2 && Modal.message({ message: '基本提示图标', status: 'info' })
if (mode === 3) {
this.status = 'info'
this.show = true
status.value = 'info'
show.value = true
}
}
function successClick(mode) {
mode === 1 && TinyModal.alert({ message: '成功提示图标', status: 'success' })
mode === 2 && TinyModal.message({ message: '成功提示图标', status: 'success' })
mode === 1 && Modal.alert({ message: '成功提示图标', status: 'success' })
mode === 2 && Modal.message({ message: '成功提示图标', status: 'success' })
if (mode === 3) {
this.status = 'success'
this.show = true
status.value = 'success'
show.value = true
}
}
function warningClick(mode) {
mode === 1 && TinyModal.alert({ message: '警告提示图标', status: 'warning' })
mode === 2 && TinyModal.message({ message: '警告提示图标', status: 'warning' })
mode === 1 && Modal.alert({ message: '警告提示图标', status: 'warning' })
mode === 2 && Modal.message({ message: '警告提示图标', status: 'warning' })
if (mode === 3) {
this.status = 'warning'
this.show = true
status.value = 'warning'
show.value = true
}
}
function errorClick(mode) {
mode === 1 && TinyModal.alert({ message: '错误提示图标', status: 'error' })
mode === 2 && TinyModal.message({ message: '错误提示图标', status: 'error' })
mode === 1 && Modal.alert({ message: '错误提示图标', status: 'error' })
mode === 2 && Modal.message({ message: '错误提示图标', status: 'error' })
if (mode === 3) {
this.status = 'error'
this.show = true
status.value = 'error'
show.value = true
}
}
function loadingClick(mode) {
mode === 1 && TinyModal.alert({ message: '加载提示图标', status: 'loading' })
mode === 2 && TinyModal.message({ message: '加载提示图标', status: 'loading' })
mode === 1 && Modal.alert({ message: '加载提示图标', status: 'loading' })
mode === 2 && Modal.message({ message: '加载提示图标', status: 'loading' })
if (mode === 3) {
this.status = 'loading'
this.show = true
status.value = 'loading'
show.value = true
}
const STATUS_MESSAGES = {
info: '基本提示图标',
success: '成功提示图标',
warning: '警告提示图标',
error: '错误提示图标',
loading: '加载提示图标'
}
function handleStatusClick(statusType, mode) {
const message = STATUS_MESSAGES[statusType]
switch(mode) {
case 1:
Modal.alert({ message, status: statusType })
break
case 2:
Modal.message({ message, status: statusType })
break
case 3:
status.value = statusType
show.value = true
break
}
}
const infoClick = (mode) => handleStatusClick('info', mode)
const successClick = (mode) => handleStatusClick('success', mode)
const warningClick = (mode) => handleStatusClick('warning', mode)
const errorClick = (mode) => handleStatusClick('error', mode)
const loadingClick = (mode) => handleStatusClick('loading', mode)

Comment on lines +32 to +37
import { Button, Modal } from '@opentiny/vue'

export default {
components: {
TinyButton,
TinyModal
TinyButton: Button,
TinyModal: Modal
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Component registration doesn't match template usage

There's an inconsistency between how components are registered and used. The template uses kebab-case tags (tiny-button, tiny-modal) but the components are registered with different names (Button, Modal).

Apply this diff to maintain consistency:

<template>
-  <tiny-button @click="infoClick(1)">
+  <Button @click="infoClick(1)">
   <!-- Update all button instances similarly -->
-  <tiny-modal v-model="show" :status="status"
+  <Modal v-model="show" :status="status"
</template>

<script>
import { Button, Modal } from '@opentiny/vue'

export default {
  components: {
-   TinyButton: Button,
-   TinyModal: Modal
+   Button,
+   Modal
  },

Committable suggestion skipped: line range outside the PR's diff.

@zzcr zzcr merged commit 56059f3 into dev Nov 28, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants