Skip to content

fix: [grid] release319, update website to fix layout in small screen #2493

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 7, 2024
Merged

fix: [grid] release319, update website to fix layout in small screen #2493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a0954b7
fix(site): fix the issue the official website is not fully displayed …
shenjunjian Nov 7, 2024
6a363c1
fix(playground): fix the issue, the playground site is not fully dis…
shenjunjian Nov 7, 2024
2df1fe2
fix(site): fix
shenjunjian Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions examples/sites/playground/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import GitHub from './icons/Github.vue'
import Share from './icons/Share.vue'

const VERSION = 'tiny-vue-version-3.19'
const NOTIFY = 'tiny-vue-playground-notify'
const NOTIFY_KEY = 'tiny-vue-playground-notify'
const LAYOUT = 'playground-layout'
const LAYOUT_REVERSE = 'playground-layout-reverse'

Expand Down Expand Up @@ -41,10 +41,10 @@ const fileDelimiter = cdnHost.includes('npmmirror') ? 'files/' : ''

let notify
const showNotify = () => {
if (localStorage.getItem(NOTIFY) !== 'true' && !notify) {
if (localStorage.getItem(NOTIFY_KEY) !== 'true' && !notify) {
const muteNotify = () => {
notify.close()
localStorage.setItem(NOTIFY, true)
localStorage.setItem(NOTIFY_KEY, true)
}
notify = Notify({
type: 'info',
Expand Down Expand Up @@ -288,22 +288,22 @@ function share() {

<template>
<div class="header">
<div class="title"><img class="logo" :src="logoUrl" /> <span>OpenTiny Vue 演练场</span></div>
<div class="title"><img class="logo" :src="logoUrl" /> <span class="mobile-hide">OpenTiny Vue 演练场</span></div>
<div>
<span class="ml20">
<span class="ml20 mobile-hide">
布局方向:
<tiny-button-group
:data="state.layoutOptions"
v-model="state.layout"
@change="changeLayout"
></tiny-button-group>
</span>
<span class="ml20">
<span class="ml20 mobile-hide">
布局反转:
<tiny-switch v-model="state.layoutReverse" mini @change="changeReserve"></tiny-switch>
</span>
<span class="ml20">
OpenTiny Vue 版本:
<span class="mobile-hide">OpenTiny Vue 版本: </span>
<tiny-select
v-model="state.selectVersion"
style="width: 150px"
Expand Down Expand Up @@ -336,6 +336,12 @@ function share() {
box-sizing: border-box;
}

/** 小屏幕时隐藏 */
@media screen and (max-width: 640px) {
.mobile-hide {
display: none;
}
}
Comment on lines +339 to +344
Copy link

@coderabbitai coderabbitai bot Nov 7, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can generate the final response as we have sufficient information about the breakpoint usage across the codebase.

Consider aligning mobile breakpoint with existing patterns

The codebase predominantly uses 767px for mobile breakpoints, while your implementation uses 640px. Consider updating to 767px to maintain consistency with other mobile-specific media queries in:

  • examples/react-site/src/views/components/componentsDetailTab.vue
  • examples/react-site/src/views/components/componentsDetail.vue
  • examples/sites/src/views/components/components.vue
🔗 Analysis chain

Verify mobile breakpoint across different devices.

The mobile-responsive CSS implementation looks good. However, please verify that the 640px breakpoint works well across various mobile devices and orientations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other breakpoint definitions to ensure consistency
rg -g '*.{vue,css,scss}' '@media.*\bmax-width\b'

Length of output: 1770

#header {
display: none;
}
Expand Down
6 changes: 4 additions & 2 deletions examples/sites/src/views/components/components.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,6 @@ export default defineComponent({
flex: 1;
display: flex;
justify-content: center;
min-width: 680px;
padding: 0 40px;
}

Expand All @@ -837,7 +836,6 @@ export default defineComponent({

:deep(> .tiny-tabs__header) {
position: sticky;
top: 90px;
z-index: var(--docs-tabs-header-zindex);
background-color: #fff;

Expand Down Expand Up @@ -974,6 +972,10 @@ export default defineComponent({
column-gap: 16px;
}

.docs-content-main {
overflow: auto;
}

.cmp-container {
p {
font-size: 16px;
Expand Down
5 changes: 4 additions & 1 deletion examples/sites/src/views/layout/layout.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,12 @@ export default defineComponent({
.content-layout {
display: flex;
--layout-tree-menu-input-height: 32px;
--layout-content-main-min-width: 600px;
--layout-content-main-min-width: 200px;
--layout-content-main-max-width: 1000px;
}
@media screen and (max-width: 640px) {
--layout-content-main-min-width: 600px;
}
Comment on lines +199 to +204
Copy link

@coderabbitai coderabbitai bot Nov 7, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect responsive design logic in media query.

There appears to be a logical error in the responsive design implementation. Currently:

  • Base min-width is set to 200px
  • For screens <= 640px, min-width increases to 600px

This is counterintuitive as smaller screens are being assigned a larger minimum width than larger screens, which could worsen the layout issues on mobile devices rather than fixing them.

Consider reversing the logic to:

  .content-layout {
    display: flex;
    --layout-tree-menu-input-height: 32px;
-   --layout-content-main-min-width: 200px;
+   --layout-content-main-min-width: 600px;
    --layout-content-main-max-width: 1000px;
  }

  @media screen and (max-width: 640px) {
-   --layout-content-main-min-width: 600px;
+   --layout-content-main-min-width: 200px;
  }

This way:

  • Larger screens maintain the original 600px min-width
  • Smaller screens (<=640px) get a reduced 200px min-width, allowing for better mobile responsiveness
📝 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
--layout-content-main-min-width: 200px;
--layout-content-main-max-width: 1000px;
}
@media screen and (max-width: 640px) {
--layout-content-main-min-width: 600px;
}
--layout-content-main-min-width: 600px;
--layout-content-main-max-width: 1000px;
}
@media screen and (max-width: 640px) {
--layout-content-main-min-width: 200px;
}


.tiny-tooltip.tiny-tooltip__popper.is-light.docs-tooltip {
border: none;
Expand Down
Loading