Skip to content

Conversation

@ljt990218
Copy link
Member

1、用 window.innerHeight 给 '#app' 的高度赋值
2、去掉全局的 '.app-wrapper' top 值,改为动态赋值

@netlify
Copy link

netlify bot commented Mar 23, 2024

Deploy Preview for vvmcsr ready!

Name Link
🔨 Latest commit 1d49eb0
🔍 Latest deploy log https://app.netlify.com/sites/vvmcsr/deploys/65fedb0cb06c8a000813fceb
😎 Deploy Preview https://deploy-preview-70--vvmcsr.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 99
Accessibility: 100
Best Practices: 100
SEO: 92
PWA: 100
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

<script>
let app = document.querySelector('#app')
window.addEventListener('load', function () {
app.style.minHeight = window.innerHeight + 'px'
Copy link
Collaborator

Choose a reason for hiding this comment

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

不建议在应用入口处设置监听事件

router.beforeEach((to) => {
to.meta.title !== undefined ? appSrapperTop.value = '46px' : appSrapperTop.value = '0px'
to.meta.title !== undefined ? appSrapperHeight.value = 'calc( 100% - 46px )' : appSrapperHeight.value = 'calc( 100% - 0px )'
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

这段代码块可能不利于后期维护


<template>
<main p="x4 y16" text-18 text="center gray-300 dark:gray-200">
<main p="x4 y62" text-18 text="center gray-300 dark:gray-200">
Copy link
Collaborator

@cwandev cwandev Mar 24, 2024

Choose a reason for hiding this comment

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

这里为什么要修改呢

Copy link
Member Author

Choose a reason for hiding this comment

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

在我的PR中,我把没有导航栏的页面 .app-wrapper的top值设为0,所以增加了 404.vue的padding值

Copy link
Collaborator

Choose a reason for hiding this comment

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

了解

<VanCell center title="🌗 暗黑模式">
<template #right-icon>
<VanSwitch v-model="checked" size="20px" aria-label="on/off Dark Mode" @click="toggle()" />
<div class="m-t-46">
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个外层容器也可去掉

@cwandev
Copy link
Collaborator

cwandev commented Mar 24, 2024

@ljt990218 谢谢这次PR,给了我很大的启发。

以上是我对本次提交代码做的 Review,可以在优化下。

  • 我为了测试生产构建版本,提交了一次代码,你需要在本地解决下冲突再提交。

此外,针对这个问题我找到了一个更好的方式实现,咱们可以探讨。

/app.less

html, body {
  height: 100%;
  overflow: hidden; /* 禁止滚动条 */
}

#app {
  height: 100%;
  position: relative;
  overflow-x: hidden;
  overflow-y: auto;
}

/App.vue

.app-wrapper {
  width: 100%;
  position: absolute;
  top: 46px;
  left: 0;
}

这种方案可能是更好的选择,因为它会更有效地利用浏览器的渲染引擎,并且在性能和可维护性方面更有优势。

@ljt990218
Copy link
Member Author

@ljt990218 谢谢这次PR,给了我很大的启发。

以上是我对本次提交代码做的 Review,可以在优化下。

  • 我为了测试生产构建版本,提交了一次代码,你需要在本地解决下冲突再提交。

此外,针对这个问题我找到了一个更好的方式实现,咱们可以探讨。

/app.less

html, body {
  height: 100%;
  overflow: hidden; /* 禁止滚动条 */
}

#app {
  height: 100%;
  position: relative;
  overflow-x: hidden;
  overflow-y: auto;
}

/App.vue

.app-wrapper {
  width: 100%;
  position: absolute;
  top: 46px;
  left: 0;
}

这种方案可能是更好的选择,因为它会更有效地利用浏览器的渲染引擎,并且在性能和可维护性方面更有优势。

OK
后面我测试,我的方案也有问题,因为是监听路由变化动态设置样式的,所以页面的布局会在路由跳转前后有跳动

@cwandev
Copy link
Collaborator

cwandev commented Mar 24, 2024

@ljt990218 谢谢这次PR,给了我很大的启发。

以上是我对本次提交代码做的 Review,可以在优化下。

  • 我为了测试生产构建版本,提交了一次代码,你需要在本地解决下冲突再提交。

此外,针对这个问题我找到了一个更好的方式实现,咱们可以探讨。

/app.less

html, body {
  height: 100%;
  overflow: hidden; /* 禁止滚动条 */
}

#app {
  height: 100%;
  position: relative;
  overflow-x: hidden;
  overflow-y: auto;
}

/App.vue

.app-wrapper {
  width: 100%;
  position: absolute;
  top: 46px;
  left: 0;
}

这种方案可能是更好的选择,因为它会更有效地利用浏览器的渲染引擎,并且在性能和可维护性方面更有优势。

OK 后面我测试,我的方案也有问题,因为是监听路由变化动态设置样式的,所以页面的布局会在路由跳转前后有跳动

我也发现了,你用这个方案再看下,辛苦!等待你的PR 😁

@ljt990218 ljt990218 closed this by deleting the head repository Mar 24, 2024
@cwandev cwandev mentioned this pull request Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants