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(autocomplete): [autocomplete] update autocomplete demos #2384

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

zzcr
Copy link
Member

@zzcr zzcr commented Oct 24, 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

  • New Features

    • Enhanced the autocomplete component with multiple size variations: "medium," "default," "small," and "mini."
    • Updated demo descriptions to clarify selectable size options for users.
  • Bug Fixes

    • Improved layout and styling for the autocomplete demo to ensure better visual clarity.
  • Tests

    • Streamlined tests to focus on validating the CSS height properties of different size elements in the autocomplete component.

Copy link

coderabbitai bot commented Oct 24, 2024

Walkthrough

The changes in this pull request involve updates to the tiny-autocomplete component across multiple files. The component now showcases multiple size variations: "medium," "default," "small," and "mini," each wrapped in a <div> with a title. The CSS has been adjusted for layout improvements, while the functionality and method bindings remain unchanged. Additionally, the test cases have been modified to validate the CSS height properties of these size variations rather than user interactions. The demo documentation has also been updated to reflect these changes.

Changes

File Path Change Summary
examples/sites/demos/pc/app/autocomplete/size-composition-api.vue, examples/sites/demos/pc/app/autocomplete/size.vue Updated the template structure to include multiple instances of tiny-autocomplete for different sizes, with new CSS for layout.
examples/sites/demos/pc/app/autocomplete/size.spec.ts Modified test case to verify CSS height properties of various autocomplete size elements instead of user interactions.
examples/sites/demos/pc/app/autocomplete/webdoc/autocomplete.js Updated demo properties for demoId: 'size' to reflect changes in selectable size values and descriptions.

Possibly related PRs

Suggested labels

enhancement

🐰 In a world of sizes, oh so bright,
Tiny-autocomplete takes flight!
From medium to mini, all in a row,
With titles and styles, watch them glow!
A hop and a skip, the heights align,
In this demo, everything's just fine! 🐇✨


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.

@zzcr zzcr changed the title Update autocomplete demo feat(autocomplete): [autocomplete] update autocomplete demos Oct 24, 2024
@github-actions github-actions bot added the enhancement New feature or request (功能增强) label Oct 24, 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: 6

🧹 Outside diff range and nitpick comments (3)
examples/sites/demos/pc/app/autocomplete/size.spec.ts (1)

12-15: Consider adding more comprehensive test coverage.

While the height assertions are good, consider adding tests to verify:

  1. The input fields are actually rendered and visible
  2. The autocomplete functionality works correctly for each size variant
  3. The dropdown positioning is correct for each size

Example additional assertions:

// Verify input fields are visible
await expect(mediumSize.locator('input')).toBeVisible()

// Verify dropdown positioning
await mediumSize.locator('input').click()
await expect(mediumSize.locator('.tiny-autocomplete__dropdown')).toBeVisible()
examples/sites/demos/pc/app/autocomplete/size-composition-api.vue (1)

5-10: Consider adding ARIA labels for better accessibility.

The autocomplete components could benefit from proper ARIA labels to improve accessibility. Consider adding aria-label attributes that describe the size of each autocomplete.

       <tiny-autocomplete
         size="medium"
         v-model="value"
         :fetch-suggestions="querySearch"
         placeholder="请输入内容"
+        aria-label="Medium sized autocomplete"
       ></tiny-autocomplete>

Also applies to: 14-14, 18-23, 27-32

examples/sites/demos/pc/app/autocomplete/size.vue (1)

3-33: Consider improving accessibility

The autocomplete demos could benefit from improved accessibility:

  1. Add aria-label to each autocomplete instance
  2. Ensure sufficient color contrast for title text

Example improvement:

-      <tiny-autocomplete
+      <tiny-autocomplete
+        :aria-label="`${size} size autocomplete demo`"
         :size="size"
         v-model="value"
         :fetch-suggestions="querySearch"
         placeholder="请输入内容"
       ></tiny-autocomplete>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 365c7e8 and 969c104.

📒 Files selected for processing (4)
  • examples/sites/demos/pc/app/autocomplete/size-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/autocomplete/size.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/autocomplete/size.vue (2 hunks)
  • examples/sites/demos/pc/app/autocomplete/webdoc/autocomplete.js (1 hunks)
🔇 Additional comments (4)
examples/sites/demos/pc/app/autocomplete/size-composition-api.vue (2)

93-98: LGTM! Well-structured CSS.

The added styles provide consistent spacing and alignment. The selectors are properly scoped and follow good CSS practices.


7-7: Verify the shared v-model behavior.

All autocomplete instances are bound to the same value model. This means changing one autocomplete will affect all others. Consider using separate v-models if independent values are needed.

Also applies to: 14-14, 20-20, 29-29

examples/sites/demos/pc/app/autocomplete/size.vue (1)

100-105: LGTM! CSS changes improve layout and readability

The added styles provide appropriate spacing and alignment for the demo layout.

examples/sites/demos/pc/app/autocomplete/webdoc/autocomplete.js (1)

32-32: LGTM! Name change improves consistency.

The simplified Chinese name '尺寸' better aligns with the component's documentation style.

await page.getByRole('option', { name: 'WWWW科技YX公司' }).click()
await expect(page.locator('//div[@class="tiny-input tiny-input-medium"]')).toHaveClass(/tiny-input-medium/)
const demo = page.locator('#size')
const meiumSize = demo.locator('.tiny-autocomplete').first()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in variable name: "meiumSize" should be "mediumSize".

The variable name contains a spelling error.

- const meiumSize = demo.locator('.tiny-autocomplete').first()
+ const mediumSize = demo.locator('.tiny-autocomplete').first()
📝 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
const meiumSize = demo.locator('.tiny-autocomplete').first()
const mediumSize = demo.locator('.tiny-autocomplete').first()

Comment on lines +7 to +10
const meiumSize = demo.locator('.tiny-autocomplete').first()
const defaultSize = demo.locator('.tiny-autocomplete').nth(1)
const smallSize = demo.locator('.tiny-autocomplete').nth(2)
const miniSize = demo.locator('.tiny-autocomplete').nth(3)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use more robust selectors instead of order-dependent ones.

The current selectors rely on element order which can be fragile if the DOM structure changes. Consider using more specific selectors based on data attributes or size classes.

- const meiumSize = demo.locator('.tiny-autocomplete').first()
- const defaultSize = demo.locator('.tiny-autocomplete').nth(1)
- const smallSize = demo.locator('.tiny-autocomplete').nth(2)
- const miniSize = demo.locator('.tiny-autocomplete').nth(3)
+ const mediumSize = demo.locator('[data-size="medium"]')
+ const defaultSize = demo.locator('[data-size="default"]')
+ const smallSize = demo.locator('[data-size="small"]')
+ const miniSize = demo.locator('[data-size="mini"]')

Committable suggestion was skipped due to low confidence.

></tiny-autocomplete>
</div>
<div>
<div class="title">small:</div>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect title text for mini size.

The title text shows "small:" for the mini-sized autocomplete component.

-      <div class="title">small:</div>
+      <div class="title">mini:</div>
📝 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
<div class="title">small:</div>
<div class="title">mini:</div>

Comment on lines +3 to +33
<div>
<div class="title">medium:</div>
<tiny-autocomplete
size="medium"
v-model="value"
:fetch-suggestions="querySearch"
placeholder="请输入内容"
></tiny-autocomplete>
</div>
<div>
<div class="title">default:</div>
<tiny-autocomplete v-model="value" :fetch-suggestions="querySearch" placeholder="请输入内容"></tiny-autocomplete>
</div>
<div>
<div class="title">small:</div>
<tiny-autocomplete
size="small"
v-model="value"
:fetch-suggestions="querySearch"
placeholder="请输入内容"
></tiny-autocomplete>
</div>
<div>
<div class="title">small:</div>
<tiny-autocomplete
size="mini"
v-model="value"
:fetch-suggestions="querySearch"
placeholder="请输入内容"
></tiny-autocomplete>
</div>
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 refactoring repeated component structure

The current implementation has significant code duplication. Consider refactoring using a v-for loop with a configuration array.

<template>
  <div class="demo-autocomplete">
-    <div>
-      <div class="title">medium:</div>
-      <tiny-autocomplete
-        size="medium"
-        v-model="value"
-        :fetch-suggestions="querySearch"
-        placeholder="请输入内容"
-      ></tiny-autocomplete>
-    </div>
-    <!-- ... other repeated blocks ... -->
+    <div v-for="size in sizes" :key="size">
+      <div class="title">{{ size }}:</div>
+      <tiny-autocomplete
+        :size="size === 'default' ? undefined : size"
+        v-model="value"
+        :fetch-suggestions="querySearch"
+        placeholder="请输入内容"
+      ></tiny-autocomplete>
+    </div>
  </div>
</template>

Add to the data section:

data() {
  return {
    restaurants: [],
    value: '',
    sizes: ['medium', 'default', 'small', 'mini']
  }
}

></tiny-autocomplete>
</div>
<div>
<div class="title">small:</div>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect title text for mini size variant

The title shows "small:" but this section demonstrates the "mini" size variant.

-      <div class="title">small:</div>
+      <div class="title">mini:</div>
📝 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
<div class="title">small:</div>
<div class="title">mini:</div>

Comment on lines +36 to 38
'zh-CN':
'<p>可选择值为<code>medium</code>,<code>default</code>,<code>small</code>,<code>mini</code>,不传递就是默认尺寸。</p>',
'en-US': '<p>The value can be <code>medium</code>, <code>small</code>, or <code>mini</code>.</p>'
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update English description to match Chinese changes.

The Chinese description has been updated to include all size options (medium, default, small, mini) and clarifies the default behavior. However, the English description is outdated and missing this information.

Apply this change to sync the descriptions:

  'zh-CN':
    '<p>可选择值为<code>medium</code>,<code>default</code>,<code>small</code>,<code>mini</code>,不传递就是默认尺寸。</p>',
- 'en-US': '<p>The value can be <code>medium</code>, <code>small</code>, or <code>mini</code>.</p>'
+ 'en-US': '<p>The value can be <code>medium</code>, <code>default</code>, <code>small</code>, or <code>mini</code>. If not specified, the default size will be used.</p>'
📝 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
'zh-CN':
'<p>可选择值为<code>medium</code>,<code>default</code>,<code>small</code>,<code>mini</code>,不传递就是默认尺寸。</p>',
'en-US': '<p>The value can be <code>medium</code>, <code>small</code>, or <code>mini</code>.</p>'
'zh-CN':
'<p>可选择值为<code>medium</code>,<code>default</code>,<code>small</code>,<code>mini</code>,不传递就是默认尺寸。</p>',
'en-US': '<p>The value can be <code>medium</code>, <code>default</code>, <code>small</code>, or <code>mini</code>. If not specified, the default size will be used.</p>'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request (功能增强)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant