fix(time-select): [time-select] Fix and optimize issues with invalid … #2735
Conversation
…input data and backfill data, and add input filtering functionality
WalkthroughThe pull request introduces enhancements to the time-select component across multiple files. The changes include modifications to test scripts, state management, filtering logic, and styling. The updates focus on improving the time selection functionality by adding input filtering, validation mechanisms, and updating the UI to handle scenarios with no matching time options. The changes span test configurations, renderless logic, Vue components, and Less stylesheets. Changes
Sequence DiagramsequenceDiagram
participant User
participant TimeSelect
participant Picker
participant TimePanel
User->>TimeSelect: Input time
TimeSelect->>Picker: Handle input
Picker->>TimePanel: Filter items
TimePanel-->>Picker: Filtered items
Picker-->>TimeSelect: Update display
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request addresses issues with invalid input data and backfill data in the time-select component. It introduces input filtering functionality to enhance data handling and user interaction. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/renderless/src/time-panel/index.ts (1)
109-117: Conditional filtering logic introduced.
Whenstate.isFilteris true, filtering is performed based onstate.filterVal. This approach is straightforward; however, consider these minor suggestions:
- Confirm
state.filterValis always a non-null string (avoid runtime errors with.includes).- Optionally, provide case-insensitive matching or more advanced matching rules if scenario demands.
packages/renderless/src/time-panel/vue.ts (1)
50-51: Consider adding TypeScript type definitions for new state propertiesThe new state properties look good, but consider adding type definitions to improve type safety and documentation.
const state = reactive({ - isFilter: false, - filterVal: '', + isFilter: false as boolean, + filterVal: '' as string,packages/vue/src/time-panel/src/pc.vue (1)
22-37: Consider extracting the item template to a separate componentThe item rendering logic is correct, but it could be extracted to improve maintainability and reusability.
Consider creating a new component like
TimeSelectItem.vue:<!-- TimeSelectItem.vue --> <template> <div class="tiny-time-select__item" :class="{ selected: selected, disabled: disabled, default: isDefault }" :disabled="disabled" @click="$emit('click', item)" > {{ item.value }} </div> </template> <script> export default { props: { item: { type: Object, required: true }, selected: { type: Boolean, default: false }, disabled: { type: Boolean, default: false }, isDefault: { type: Boolean, default: false } } } </script>packages/theme/src/time-select/index.less (2)
23-25: Consider using min-height instead of fixed heightThe fixed height of 36px might be too rigid. Using
min-heightwould allow the container to grow if needed while maintaining the minimum space required.&.noMatchHeight { - height: 36px; + min-height: 36px; }
61-63: Consider adding vertical padding for better spacingThe centered text looks good, but consider adding some vertical padding for better visual balance when there are no matches.
.noMatch { text-align: center; + padding: 8px 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/sites/demos/pc/app/time-select/range-placeholder.spec.ts(1 hunks)packages/renderless/src/picker/index.ts(3 hunks)packages/renderless/src/time-panel/index.ts(1 hunks)packages/renderless/src/time-panel/vue.ts(1 hunks)packages/theme-saas/src/time-select/index.less(2 hunks)packages/theme/src/time-select/index.less(2 hunks)packages/vue/src/time-panel/src/pc.vue(1 hunks)
🔇 Additional comments (15)
examples/sites/demos/pc/app/time-select/range-placeholder.spec.ts (4)
11-11: Adjustment of maximum time index seems correct.
Increasing the index from43to44likely aligns with the newly computed list size of time options.
13-13: Updated selection index for start time.
Switching the click action fromoptions.nth(9)tooptions.nth(10)reflects the added option in the list.
20-20: Adjustment of end-time maximum index is consistent.
Increasing the index to44ensures alignment with the newly lengthened time list.
22-23: Disabled check and selection of subsequent time slot.
Verifies thatoptions.nth(10)is disabled before clickingnth(11)for selection. This helps ensure correct time boundaries.packages/renderless/src/picker/index.ts (3)
548-552: User input filtering for time-select.
SettingisFiltertotrueand assigningfilterValtouserInputdynamically adjusts the time panel items. This is a clean approach; ensure large data sets remain performant if time options are numerous.
995-999: Validating time-select user input upon Enter key.
This regex check restricts user entries to0-24, and if valid, uses the first matching item. Consider clarifying edge cases like "24:00" or partially entered times ("2").
1024-1027: Resetting filter state on picker hide.
Clearingstate.isFilterprevents stale filtering on the next open, which is good. Keep in mind potential scenarios if the user reopens shortly and expects the prior filter — but logically, this reset is consistent with typical UX patterns.packages/theme-saas/src/time-select/index.less (5)
4-5: New prefix variables defined.
The introduction of@picker-panel-prefix-clsand@time-select-scrollbar-clskeeps naming consistent and flexible.
9-11:.noMatchHeightclass addition.
Setting a height of36pxfor the no-match scenario helps maintain a consistent layout.
12-13: Height and top padding for time-select container.
A242pxheight with8pxtop padding likely ensures correct spacing for the new item listing.
14-16: Scrollbar height adjustment.
Ensuring the scrollbar takes full height is a neat improvement for usability.
53-59: Styling for.noMatchstate.
Centering text for the no-match condition (text-align: center) is good for user feedback.packages/vue/src/time-panel/src/pc.vue (2)
18-18: LGTM! Good use of dynamic class bindingThe conditional class binding is well implemented, providing appropriate styling feedback based on the items state.
38-38: LGTM! Good use of internationalizationThe "no match" message is properly internationalized using the translation system.
packages/theme/src/time-select/index.less (1)
27-27: LGTM! Good improvements to layout flexibilityThe use of CSS variables for padding and changing scrollbar height to 100% improves the component's flexibility and maintainability.
Also applies to: 29-29
…input data and backfill data, and add input filtering functionality
PR
fix(time-select): [time-select] 修复了简易输入时间，回车后不补充完整时间的问题。此问题之前是没有考虑处理的，并且输入框还可以输入非法内容，属于遗漏了检验数据的功能。这次修改增加了输入时间，选择面板数据会自动过滤出相符合的时间，回车后可以进行回填补充，默认补充第一条匹配的数据。在输入非法字段后回车会清空无效数据，也增加了一个过滤为空的‘暂无匹配数据’的展示。
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #1247
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests