Skip to content
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: Menu enables vue-router mode #7681

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion frontend/src/components/layout-content/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</template>
</back-button>

<span v-else class="flex justify-between items-start sm:items-center flex-col sm:flex-row">
<span v-else>
{{ title }}
<span v-if="slots.buttons">
<el-divider direction="vertical" />
Copy link
Member

Choose a reason for hiding this comment

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

There is no visible issue in the code snippet provided between lines 36 and 40 of the difference file. The changes appear to be simply refactoring the v-if statement, which should not affect functionality or layout.

If you want me to perform more thorough checks or provide additional suggestions based on broader code context or specific requirements, please let me know!

Expand Down
23 changes: 5 additions & 18 deletions frontend/src/layout/components/Sidebar/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@
<el-scrollbar>
<el-menu
:default-active="activeMenu"
:router="menuRouter"
:router="true"
:collapse="isCollapse"
:collapse-transition="false"
:unique-opened="true"
@select="handleMenuClick"
class="custom-menu"
>
<SubItem :menuList="routerMenus" />
<el-menu-item :index="''">
<el-icon @click="logout">
<el-menu-item :index="''" @click="logout">
<el-icon>
<SvgIcon :iconName="'p-logout'" />
</el-icon>
<template #title>
<span @click="logout">{{ $t('commons.login.logout') }}</span>
<span>{{ $t('commons.login.logout') }}</span>
</template>
</el-menu-item>
</el-menu>
Expand Down Expand Up @@ -55,13 +55,7 @@ import PrimaryMenu from '@/assets/images/menu-bg.svg?component';
const route = useRoute();
const menuStore = MenuStore();
const globalStore = GlobalStore();
defineProps({
menuRouter: {
type: Boolean,
default: true,
required: false,
},
});

const activeMenu = computed(() => {
const { meta, path } = route;
return isString(meta.activeMenu) ? meta.activeMenu : path;
Expand Down Expand Up @@ -201,13 +195,6 @@ onMounted(() => {
line-height: normal;
}

.custom-menu .el-menu-item {
white-space: normal !important;
word-break: break-word;
overflow-wrap: break-word;
line-height: normal;
}

.sidebar-container {
position: relative;
display: flex;
Copy link
Member

Choose a reason for hiding this comment

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

No significant issues were found; however, there are some minor improvements that can be made to enhance readability and maintainability:

  1. Simplify Props Definition: The menuRouter prop has been removed as it's not being used inside the component. This line of code should be commented out if you want to remove it permanently.

-defineProps({

  • menuRouter: {
  •    type: Boolean,
    
  •    default: true,
    
  •    required: false,
    
  • },
    -});

2. **Remove Extra White Space in CSS Rule**: There seems to be extra whitespace at the beginning of the `.custom-menu .el-menu-item` rule, which might lead to unintended behavior. It should be cleaned up.

   ```diff
--.custom-menu .el-menu-item {
-    white-space: normal !important;
-    word-break: break-word;
-    overflow-wrap: break-word;
-    line-height: normal;
 }

These changes will make the code cleaner and more readable without introducing any functionality loss.

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/layout/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div :class="classObj" class="app-wrapper" v-loading="loading" :element-loading-text="loadingText" fullscreen>
<div v-if="classObj.mobile && classObj.openSidebar" class="drawer-bg" @click="handleClickOutside" />
<div class="app-sidebar" v-if="!globalStore.isFullScreen">
<Sidebar @menu-click="handleMenuClick" :menu-router="!classObj.openMenuTabs" />
<Sidebar @menu-click="handleMenuClick" />
</div>

<div class="main-container">
Copy link
Member

Choose a reason for hiding this comment

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

There is an inconsistency between the conditional rendering of both Sidebar components based on the values of classObj.mobile, classObj.openSidebar, and globalStore.isFullScreen. In one instance (v-if="!globalStore.isFullScreen"), you're conditionally showing the Sidebar while also checking if either classObj.mobile or classObj.openSidebar is true. This could result in multiple instances of the Sidebar being rendered in certain scenarios.

Here's a revised version with consistent logic:

@@ -2,7 +2,7 @@
     <div :class="classObj" class="app-wrapper" v-loading="loading" :element-loading-text="loadingText" fullscreen>
         <div v-if="classObj.mobile && classObj.openSidebar" class="drawer-bg" @click="handleClickOutside" />
         <div class="app-sidebar" :key="sidebarKey"
-          v-if=" !classObj.mobile || globalStore.isFullScreen">
+             v-if=" globalStore.isFullScreen || (classObj.mobile && classObj.openSideBar) ">

In this revision, I've consolidated the conditions so that only one instance of Sidebar is displayed under normal usage where neither classObj.mobile nor globalStore.isFullScreen is true. The sidebar remains visible when the screen is not full but user settings permit it to be open using classObj.openMenuTabs.

Additionally, adding a unique key (:key="sidebarKey") helps React manage re-rendering correctly for dynamic content updates. Ensure that there exists at least one initial state value assignable to sidebarKey.

After making these changes, ensure that your Vuex store (globalStore) correctly stores the appropriate states related to mobile view toggles and sidebars.

Expand Down
2 changes: 1 addition & 1 deletion frontend/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export default defineConfig(({ mode }: ConfigEnv): UserConfig => {
],
esbuild: {
pure: viteEnv.VITE_DROP_CONSOLE ? ['console.log'] : [],
drop: viteEnv.VITE_DROP_CONSOLE ? ['debugger'] : [],
drop: viteEnv.VITE_DROP_CONSOLE && process.env.NODE_ENV === 'production' ? ['debugger'] : [],
},
build: {
outDir: '../cmd/server/web',
Expand Down
Loading