-
Notifications
You must be signed in to change notification settings - Fork 279
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: optimize the example display style of the official website #2624
Conversation
WalkthroughThe changes in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
examples/sites/src/views/components/demo.vue (3)
316-318
: Consider using CSS variables and consistent border radius values.The styling could be improved for better maintainability and consistency:
.pc-demo-container { - background: #fff; - border-radius: 6px; - border: 1px solid #dcdfe6; + background: var(--ti-demo-container-bg, #fff); + border-radius: var(--ti-border-radius-sm); + border: 1px solid var(--ti-border-color, #dcdfe6);
Line range hint
73-91
: Add error handling for clipboard operations.The clipboard operations should check for browser support and handle errors gracefully.
async copyCode(demo) { + if (!navigator.clipboard) { + console.error('Clipboard API not supported'); + return; + } if (demo.isOpen) { const idx = parseInt(state.tabValue.slice(3)) - navigator.clipboard.writeText(demo.files[idx].code) + try { + await navigator.clipboard.writeText(demo.files[idx].code) + } catch (err) { + console.error('Failed to copy code:', err); + return; + } } else { await this.getDemoCode(demo) - navigator.clipboard.writeText(demo.files[0].code) + try { + await navigator.clipboard.writeText(demo.files[0].code) + } catch (err) { + console.error('Failed to copy code:', err); + return; + } } state.copyTip = i18nByKey('copyCodeOk') state.copyIcon = 'i-ti-check' }
Line range hint
379-384
: Consider using responsive units and importing background image as an asset.The mobile view container uses hardcoded pixel values which may not work well across different devices.
.mobile-view-container { - width: 360px; - height: 613px; + width: min(360px, 90vw); + height: min(613px, 80vh); position: absolute; left: 11px; top: 79px;Also, consider importing the phone frame as an asset rather than using a base64 encoded image in the CSS. This would improve maintainability and reduce the CSS file size.
Consider moving the phone frame image to an assets directory and importing it:
import phoneFrame from '@/assets/images/phone-frame.png';Then use it in the style:
- background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAA...) + background: url(${phoneFrame})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
examples/sites/src/views/components/demo.vue
(1 hunks)
🔇 Additional comments (1)
examples/sites/src/views/components/demo.vue (1)
Line range hint 1-424
: Verify consistent usage of the demo component across the site.
Let's ensure that the changes to the demo component don't break existing implementations.
✅ Verification successful
Based on the analysis of the shell script outputs, I can now provide a final response:
The demo component and its styling are consistently used across both Vue and React sites
The verification shows that:
- The demo component is used consistently in both Vue and React sites with similar patterns
- The mobile container styles (
phone-container
andmobile-view-container
) are reused across multiple components with identical styling - The changes to the demo component in the review are safe as they maintain compatibility with existing implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of the demo component
echo "Checking demo component usage patterns..."
rg -A 2 '<demo\s+[^>]*>' --type vue
# Check for any hardcoded references to old styling
echo "Checking for hardcoded style references..."
rg -A 2 'pc-demo-container|mobile-view-container' --type vue --type css
Length of output: 355
Script:
#!/bin/bash
# Search for demo component usage with file extension
echo "Checking demo component usage patterns..."
fd -e .vue -x rg -l '<demo\s+[^>]*>'
# Check for style references in vue files
echo "Checking style references in vue files..."
fd -e .vue -x rg -l 'pc-demo-container|mobile-view-container'
# Check imports of demo component
echo "Checking demo component imports..."
fd -e .vue -x rg -l "from.*demo.vue"
Length of output: 779
Script:
#!/bin/bash
# Check the content of files using the demo component
echo "Checking demo component usage in components.vue..."
cat ./examples/sites/src/views/components/components.vue
echo "Checking demo component usage in componentsDetail.vue..."
cat ./examples/react-site/src/views/components/componentsDetail.vue
echo "Checking demo component usage in componentsDetailTab.vue..."
cat ./examples/react-site/src/views/components/componentsDetailTab.vue
# Check the content of files with style references
echo "Checking mobile.vue style references..."
cat ./examples/docs/newsrc/mobile.vue
echo "Checking demoPage.vue style references..."
cat ./examples/react-site/src/views/components/demoPage.vue
Length of output: 80308
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores