feat(bulletin-board): [bulletin-board] Add click events #2143 #2601
Conversation
WalkthroughThe changes in this pull request involve significant updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
wuyiping0628
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (11)
examples/sites/demos/pc/app/bulletin-board/events.spec.ts (1)
3-11: Test coverage could be enhancedWhile the basic click event test is good, consider adding:
- Multiple test cases with different bulletin items
- Negative test cases (e.g., disabled items)
- Modal dismissal verification
test('测试点击事件', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('bulletin-board#events') const demo = page.locator('#events') const modal = page.locator('.tiny-modal') await demo.getByRole('link', { name: 'TINY v2.1.3 版本' }).click() await expect(modal.getByText('click 事件：内容为TINY v2.1.3 版本')).toBeVisible() + + // Test multiple items + await demo.getByRole('link', { name: 'Another Item' }).click() + await expect(modal.getByText('click 事件：内容为Another Item')).toBeVisible() + + // Verify modal can be dismissed + await page.keyboard.press('Escape') + await expect(modal).not.toBeVisible() })packages/renderless/src/bulletin-board/index.ts (1)
38-42: Add TypeScript types for better type safetyThe click handler would benefit from explicit TypeScript types for better maintainability and type checking.
+interface BulletinBoardItem { + url?: string; + content: string; + [key: string]: any; +} export const handleBulletinBoardClick = - ({ emit }) => - (item) => { + ({ emit }: { emit: (event: string, item: BulletinBoardItem) => void }) => + (item: BulletinBoardItem) => { emit('handleClick', item) }packages/renderless/src/bulletin-board/vue.ts (2)
13-15: Add JSDoc documentation for the new APIThe new handleBulletinBoardClick API should be documented for better developer experience.
+/** + * @typedef {Object} BulletinBoardItem + * @property {string} [url] - Optional URL for the bulletin item + * @property {string} content - Content of the bulletin item + */ +/** + * @type {string[]} api - List of exposed API methods + * @property {string} handleBulletinBoardClick - Event handler for bulletin item clicks + */ export const api = ['state', 'getRoute', 'handleBulletinBoardClick']
17-17: Document the emit event contractAdd documentation for the event emission contract to help consumers understand the expected payload.
+/** + * @typedef {Object} RenderlessProps + * @property {string} activeName - Currently active bulletin name + * @property {Array<BulletinBoardItem[]>} data - Bulletin board data + */ +/** + * Renderless function for bulletin board component + * @param {RenderlessProps} props - Component props + * @param {Object} context - Vue composition API context + * @param {Object} hooks - Component hooks + * @param {Function} hooks.emit - Event emitter that triggers 'handleClick' with BulletinBoardItem + */ export const renderless = (props, { reactive, computed, watch }, { emit }) => {Also applies to: 37-38
examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue (1)
89-94: Enhance click handler with type safetyThe click handler should include type information for better maintainability and IDE support.
-function handleClick(item) { +interface BulletinItem { + text: string; + date?: string; + url?: string; + target?: string; + route?: string; +} + +function handleClick(item: BulletinItem) { TinyModal.message({ message: 'click 事件：内容为' + item.text, status: 'info' }) }examples/sites/demos/pc/app/bulletin-board/events.vue (1)
97-102: Consider extracting message text to constantsThe message text is hardcoded in both components. Consider extracting it to a shared constant or i18n key.
+const CLICK_MESSAGE_PREFIX = 'click 事件：内容为'; + methods: { handleClick(item) { TinyModal.message({ - message: 'click 事件：内容为' + item.text, + message: CLICK_MESSAGE_PREFIX + item.text, status: 'info' }) } }packages/vue/src/bulletin-board/src/pc.vue (2)
Line range hint
35-41: Enhance accessibility for non-link clickable elementsThe span element that acts as a clickable item should have proper accessibility attributes.
- <span v-else class="tiny-bulletin-board__textTitle" @click.native="handleBulletinBoardClick(subItem)"> + <span + v-else + class="tiny-bulletin-board__textTitle" + @click="handleBulletinBoardClick(subItem)" + role="button" + tabindex="0" + @keydown.enter="handleBulletinBoardClick(subItem)" + >
Line range hint
28-41: Consider keyboard navigation for bulletin itemsThe bulletin board should support keyboard navigation between items for better accessibility.
Consider implementing:
- Arrow key navigation between items
- Focus management
- ARIA attributes for the current item
Would you like me to help create an issue for tracking these accessibility improvements?examples/sites/demos/apis/bulletin-board.js (1)
109-120: Consider enhancing the BulletinBoardData interface documentationWhile the interface is well-structured, the comments could be improved for better clarity.
Consider updating the interface documentation:
interface BulletinBoardData { - text: string // 显示文本 - date: string // 日期 - url: string // 需要跳转的地址 - target: string // <a> 标签的一个属性，该属性指定在何处显示链接的资源 + text: string // Display text for the bulletin item + date: string // Publication date of the bulletin + url?: string // Optional URL for redirection when clicked + target?: string // Optional target attribute for URL opening behavior (_blank, _self, etc.) }examples/sites/demos/pc/app/bulletin-board/webdoc/bulletin-board.js (2)
109-118: Consider improving English translationWhile the demo entry is well-structured, the English translation could be more natural.
Consider updating the English text:
name: { 'zh-CN': 'click事件', - 'en-US': 'Fold panel click event' + 'en-US': 'Click Event' }, desc: { 'zh-CN': '<p>当点击后会触发触发 <code>click</code> 事件。</p>', - 'en-US': '<p>When clicked, it will trigger<code>click</code>. </p>' + 'en-US': '<p>Triggers the <code>click</code> event when a bulletin item is clicked.</p>' }
115-115: Fix typo in Chinese descriptionThere's a duplicate word "触发" in the Chinese description.
-'zh-CN': '<p>当点击后会触发触发 <code>click</code> 事件。</p>', +'zh-CN': '<p>当点击后会触发 <code>click</code> 事件。</p>',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
examples/sites/demos/apis/bulletin-board.js(2 hunks)examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue(1 hunks)examples/sites/demos/pc/app/bulletin-board/events.spec.ts(1 hunks)examples/sites/demos/pc/app/bulletin-board/events.vue(1 hunks)examples/sites/demos/pc/app/bulletin-board/webdoc/bulletin-board.js(1 hunks)packages/renderless/src/bulletin-board/index.ts(1 hunks)packages/renderless/src/bulletin-board/vue.ts(2 hunks)packages/theme/src/bulletin-board/index.less(1 hunks)packages/vue/src/bulletin-board/src/pc.vue(1 hunks)
🔇 Additional comments (5)
packages/renderless/src/bulletin-board/vue.ts (1)
Line range hint 17-42: Verify event handling in component usage
Let's verify that the click event is properly handled in component implementations.
✅ Verification successful
Event handling implementation is properly integrated
The verification shows that event handling is correctly implemented:
- The component properly emits click events via
handleBulletinBoardClickfor both URL and non-URL items - Event handling is consistently implemented across different usage patterns (basic, composition API)
- The click handler is properly bound with
.nativemodifier where needed - Example implementations demonstrate correct event binding with
@handleClick - The component supports both URL navigation and route-based navigation with proper click handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check component implementations for proper event handling
# Find all Vue files using the bulletin-board component
echo "Checking component usage patterns..."
rg -l "tiny-bulletin-board" --type vue
# Check for proper event handling
echo "\nChecking event handler implementation..."
rg "@handleClick" --type vue -A 2
Length of output: 306
Script:
#!/bin/bash
# Let's try a different approach to find component usage and event handling
# Find files containing bulletin-board references
echo "Searching for bulletin-board component usage..."
rg -l "bulletin-board"
# After finding files, let's check their content for event handling
echo "\nChecking event handler implementations..."
rg -p "bulletin-board.*@" -A 3
Length of output: 8018
Script:
#!/bin/bash
# Let's examine the event handling implementation in more detail
# Check the test files for event handling verification
echo "Checking test implementations..."
rg -p "handleClick" examples/sites/demos/pc/app/bulletin-board/events.spec.ts -A 5
# Check the actual component implementation
echo "\nChecking component implementation..."
cat packages/vue/src/bulletin-board/src/pc.vue
# Check for any additional event handling in the base demo
echo "\nChecking base implementation..."
cat examples/sites/demos/pc/app/bulletin-board/base.vue
Length of output: 6091
examples/sites/demos/pc/app/bulletin-board/events.vue (1)
1-2: Event handler naming inconsistency
Same issue as in the Composition API version - event handler name should be @click instead of @handleClick.
examples/sites/demos/apis/bulletin-board.js (2)
21-26: LGTM! Type safety improvement
The change from generic Array to strongly-typed BulletinBoardData[] improves type safety and developer experience.
91-102: LGTM! Well-defined click event
The click event is well-defined with proper typing and clear documentation in both languages.
packages/theme/src/bulletin-board/index.less (1)
34-34: LGTM! Improved user interaction feedback
The addition of cursor: pointer provides appropriate visual feedback for the new click functionality.
examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
examples/sites/demos/apis/bulletin-board.js (1)
21-26: Enhance the data property descriptionsWhile the type specification is now more precise with
BulletinBoardData[], the descriptions could be more informative by including:
- The purpose of the data
- Required/optional fields
- Example usage
Consider updating the descriptions to:
- 'zh-CN': 'tab-item 数据;', - 'en-US': 'tab-item data;' + 'zh-CN': '公告栏数据列表，每项包含文本内容(text)、日期(date)、链接(url)和打开方式(target)', + 'en-US': 'Bulletin board data list. Each item contains text content, date, URL, and target attribute for link behavior'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
examples/sites/demos/apis/bulletin-board.js(2 hunks)packages/vue/src/bulletin-board/src/pc.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/bulletin-board/src/pc.vue
🔇 Additional comments (2)
examples/sites/demos/apis/bulletin-board.js (2)
91-103: LGTM! Well-structured event definition
The handleClick event is well-defined with:
- Clear type signature using BulletinBoardData
- Descriptive documentation in both languages
- Proper demo reference
Line range hint 1-122: Verify API changes against PR objectives
The changes align with the PR objective of adding click events to the bulletin board. However, let's verify the integration with existing code to ensure backward compatibility.
✅ Verification successful
API changes are consistent and backward compatible
The verification shows that:
- The
handleClickevent is already implemented and used in existing demo files (events.vueandevents-composition-api.vue) - The API documentation in
bulletin-board.jscorrectly reflects the existing implementation - The event handler follows the same pattern across both traditional and composition API usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing usage of bulletin-board component to ensure changes don't break existing code
# Search for bulletin-board component usage
echo "Checking bulletin-board component usage patterns..."
rg -l "tiny-bulletin-board" --type vue
# Search for any existing click handlers
echo "Checking existing click handler patterns..."
rg "(@click|v-on:click|handleClick).*bulletin" --type vue
Length of output: 364
Script:
#!/bin/bash
# Description: Check for existing usage of bulletin-board component and click handlers with broader search
# Search for bulletin-board component usage in all files
echo "Checking bulletin-board component usage patterns..."
rg -l "bulletin-board"
# Search for demo files related to bulletin board
echo "Checking demo files..."
fd "bulletin.*\.(vue|jsx?|tsx?)"
# Search for any existing click handlers related to bulletin
echo "Checking existing click handler patterns..."
rg "(@click|v-on:click|handleClick).*bulletin"
Length of output: 6537
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue (2)
10-88: Consider adding type annotations for reactive refsWhile the code works, adding type annotations would improve maintainability and provide better IDE support.
-const tabTitle = ref(['TINY 更新日志', '他们都在用', 'TINY 特性']) +const tabTitle = ref<string[]>(['TINY 更新日志', '他们都在用', 'TINY 特性']) -const data = ref([ +const data = ref<BulletinBoardData[][]>([
89-94: Add error handling to contentClick functionConsider adding error handling to manage cases where item properties might be undefined.
function contentClick(item) { + if (!item?.text) { + console.warn('Invalid item clicked') + return + } TinyModal.message({ message: '触发 contentClick 事件：内容为' + item.text, status: 'info' }) }examples/sites/demos/apis/bulletin-board.js (1)
21-26: Enhance data property documentationThe current description is too brief. Consider adding more details about the data structure and usage.
desc: { - 'zh-CN': 'tab-item 数据;', - 'en-US': 'tab-item data;' + 'zh-CN': 'tab-item 数据，每个数组项代表一个标签页的内容列表，内容项需符合 BulletinBoardData 接口定义', + 'en-US': 'tab-item data, each array item represents a content list for a tab, content items should conform to BulletinBoardData interface' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
examples/sites/demos/apis/bulletin-board.js(2 hunks)examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue(1 hunks)examples/sites/demos/pc/app/bulletin-board/events.vue(1 hunks)examples/sites/demos/pc/app/bulletin-board/webdoc/bulletin-board.js(1 hunks)packages/renderless/src/bulletin-board/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/renderless/src/bulletin-board/index.ts
- examples/sites/demos/pc/app/bulletin-board/events.vue
- examples/sites/demos/pc/app/bulletin-board/webdoc/bulletin-board.js
🔇 Additional comments (4)
examples/sites/demos/pc/app/bulletin-board/events-composition-api.vue (2)
1-3: LGTM! Template structure and event binding are correct.
The component usage follows Vue.js best practices with proper prop binding and event handling.
15-15: Security: URLs should use HTTPS
Several URLs in the data are using either HTTP or localhost. For production examples, use HTTPS URLs to demonstrate secure practices.
Also applies to: 20-20, 25-25, 33-33, 38-38, 44-44, 49-49, 55-55, 61-61
examples/sites/demos/apis/bulletin-board.js (2)
91-105: LGTM! Event definition is well documented
The contentClick event is properly defined with:
- Clear type definition
- Bilingual descriptions
- Version metadata
111-123: Improve the interface definition
The BulletinBoardData interface needs the following improvements:
- Add bilingual comments
- Specify types for all fields
- Fix code formatting
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2143
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style
Tests