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.

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

style: Optimize style and layout #7671

Merged
merged 2 commits into from
Jan 8, 2025
Merged

style: Optimize style and layout #7671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7168e20
style: Optimize style and layout
lan-yonghui Jan 8, 2025
3dd1db6
style: Optimize style and layout
lan-yonghui Jan 8, 2025
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/app-status/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,6 @@ defineExpose({
border: 1px solid var(--el-color-warning);
background-color: transparent;
padding: 8px 8px;
width: 78px;
max-width: 100px;
}
</style>
Copy link
Member

Choose a reason for hiding this comment

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

In the provided code snippet, there is one minor change that can be made to enhance readability:

Change from:

width: 78px;

To:

max-width: 100px;

Explanation:

  • Original Code: The width property is set directly to 78px.
  • Proposed Change: Changing it to max-width: 100px; allows the element to expand up to its maximum allowable width while maintaining responsiveness and not exceeding the specified size.

This suggests a preference for flexibility without setting an exact pixel limit.

Copy link
Member

Choose a reason for hiding this comment

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

The changes suggest that the element's width has been changed from 78px to max-width: 100px. This should not be modified without careful consideration as it might affect the layout of other elements on the page, potentially causing overflow or responsiveness problems.

2 changes: 1 addition & 1 deletion frontend/src/components/layout-content/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</template>
</back-button>

<span v-else>
<span v-else class="flex justify-between items-start sm:items-center flex-col sm:flex-row">
{{ title }}
<span v-if="slots.buttons">
<el-divider direction="vertical" />
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/components/router-button/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<el-card class="router_card">
<div class="flex w-full flex-col items-center md:justify-between md:flex-row">
<el-card class="router_card p-1 sm:p-0">
<div class="flex w-full flex-col justify-start sm:items-center items-start sm:justify-between sm:flex-row">
<el-radio-group v-model="activeName" @change="handleChange">
<el-radio-button
class="router_card_button"
Expand All @@ -15,7 +15,7 @@
</el-badge>
</el-radio-button>
</el-radio-group>
<div class="flex flex-row gap-2 md:flex-col lg:flex-row">
<div class="flex flex-col gap-2 sm:flex-row">
<slot name="route-button"></slot>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

After checking the code differences, I identified several irregularities and potential issues:

  1. HTML Tags: The flex classes are incorrectly written with hyphens (sm) instead of camel case (sm:). This should be corrected to use colon notation like sm:flex.

  2. Slot Content Placement: In the slot content, using a conditional statement directly inside it can lead to unexpected behavior because slots don't naturally support direct JavaScript logic within their contents. Consider extracting this logic outside the template if possible.

  3. Card Padding: Removing padding from .el-card might not be desired, and increasing padding could improve readability or aesthetics.

  4. Flex Direction: Aligning buttons vertically by default can change the layout structure unnecessarily. If needed, consider changing the alignment only when necessary (e.g., on screens smaller than medium).

Here's an improved version of the code addressing these points:

<template>
    <el-card class="router_card">
        <div class="w-full flex flex-grow flex-col items-center justify-end gap-y-4 sm:flex-row">
            <!-- Radio group for displaying routes -->
            <el-radio-group v-model="activeName" @change="handleChange">
                <el-radio-button 
                    class="router_card_button px-4 py-2"
                    :disabled="!showRouteButton"
                    >{route.name}
                        <span>{route.icon}</span></el-radio-button>
                </el-radio-button>        
            </el-radio-group>

            <!-- Slot for additional route buttons or actions -->
            <div class="flex flex-gap-2 lg:hidden">                
                <slot name="extra-actions"></slot>
            </div>
            
            <!-- Additional div for slot content at mobile sizes -->
            <div class="hidden md:block">
                <slot name="extra-actions"></slot>
            </div>
       </div>
   </el-card>
</template>

<script lang='ts'>
import { ElCard, ElRadioGroup, ElRadioButton } from 'element-plus';
export default {
    components: {
      ElCard,
      ElRadioGroup,
      ElRadioButton
    },
    data() {
        return {
          activeName: '',
          showRouteButton: true // Example variable controlling visibility of the button
        };
    },
    methods: {
      handleChange(name) {
         console.log('Active Name:', name);
      }
    }
};
</script>

<style scoped lang='scss'>

.router_card {
    height:auto;
    position:relative;

}

// Responsive styling examples
@media screen and (max-width:900px){
    .el-card{
     display:none; /* Adjust based on your requirements */
    }

    .btn_container {

     .btn_item{

           background-color:#fff5ec;
            margin-right:2% !important;

         &.selected , &:focus-within{
              background-image:url('@/assets/images/checked.png');
              border-bottom-left-radius:100%!important ;
              border-bottom-right-radius:100%!important ;
               border-top:1px solid black;
           }
     }
    }
    
   
}

/* Other styles as per project design */

This revised code maintains correct HTML tag formatting, improves slot usage and logic extraction, adjusts card padding according to need, and offers basic responsiveness adjustments through media queries. Make sure to customize any other styles as appropriate for your application's design needs.

Copy link
Member

Choose a reason for hiding this comment

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

  1. In the <el-card> element, consider adding padding (p-1) instead of margin for better spacing consistency.
  2. For smaller screens (sm), reduce the horizontal margins to sm:p-0. Adjusting paddings based on screen sizes helps maintain consistent visual appearance across various devices.
  3. The .md:justify-between and .lg:flex-row classes can be consolidated into a single .mx-auto class to center content more efficiently while maintaining responsiveness.

Here's the optimized solution:

<template>
    <el-card class="router_card p-1 sm:p-0">
        <div class="flex w-full flex-col justify-start sm:items-center items-start mx-auto sm:justify-between sm:flex-row">
            <el-radio-group v-model="activeName" @change="handleChange">
                ...
            </el-radio-group>
            <div class="flex flex-col gap-2 sm:flex-row">
                ...
            </div>
        </div>
    </el-card>
</template>

<script>
export default {
    data() {
        return {
            activeName: ""
        };
    },
    methods: {
        handleChange(name) {
            console.log(`Selected tab: ${name}`);
        }
    }
};
</script>

These changes improve readability and adaptibility by ensuring uniform styling and enhanced user experience across different device sizes.

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/app-store/apps/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
</el-row>
</template>
<template #rightButton>
<div class="flex justify-end">
<div class="flex justify-end flex-col sm:flex-row">
<fu-table-pagination
v-model:current-page="paginationConfig.currentPage"
v-model:page-size="paginationConfig.pageSize"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/container/container/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</el-card>
<LayoutContent :title="$t('container.container', 2)" :class="{ mask: dockerStatus != 'Running' }">
<template #rightButton>
<div class="flex justify-end">
<div class="flex justify-end flex-col sm:flex-row">
<div class="mr-10">
<el-checkbox v-model="includeAppStore" @change="search()">
{{ $t('container.includeAppstore') }}
Expand Down
34 changes: 14 additions & 20 deletions frontend/src/views/home/app/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,19 @@
<div>
<el-scrollbar height="525px" class="moz-height">
<div class="h-app-card" v-for="(app, index) in apps" :key="index">
<el-row :gutter="10">
<el-col :span="5">
<div>
<el-avatar shape="square" :size="55" :src="'data:image/png;base64,' + app.icon" />
<div class="flex justify-start items-center gap-2">
<div class="w-14">
<el-avatar shape="square" :size="55" :src="'data:image/png;base64,' + app.icon" />
</div>
<div class="flex-1 flex flex-col h-app-content">
<span class="h-app-title">{{ app.name }}</span>
<div class="h-app-desc">
<span>
{{ language == 'zh' || language == 'tw' ? app.shortDescZh : app.shortDescEn }}123123
</span>
</div>
</el-col>
<el-col :span="15">
<div class="h-app-content">
<div>
<span class="h-app-title">{{ app.name }}</span>
</div>
<div class="h-app-desc">
<span>
{{ language == 'zh' || language == 'tw' ? app.shortDescZh : app.shortDescEn }}
</span>
</div>
</div>
</el-col>
<el-col :span="2">
</div>
<div>
<el-button
class="h-app-button"
type="primary"
Expand All @@ -32,8 +26,8 @@
>
{{ $t('app.install') }}
</el-button>
</el-col>
</el-row>
</div>
</div>
<div class="h-app-divider" />
</div>
</el-scrollbar>
Copy link
Member

Choose a reason for hiding this comment

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

The updated code has several improvements that enhance readability, maintainability, and performance:

  1. Flexbox Layout: The use of display: flex replaces nested el-row and el-col to improve layout structure. This makes the code cleaner and easier to understand.

  2. Grid Removal: The <el-grid> element is removed, simplifying the divs structure for better styling manipulation.

  3. Code Refactoring:

    • A function call $t('app.install') is moved into a single line inside the button template literal.
    • Spacing adjustments are made to ensure consistent formatting around braces {} within Vue template interpolations.

  4. Optimization:

    • Removed unnecessary comments as they clutter the code without adding value to understanding its functionality.

Overall, this change enhances the overall appearance and functionality while maintaining a clean development pattern.

Copy link
Member

Choose a reason for hiding this comment

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

There are several improvements that can be made to this code:

  1. Consolidate Row Spans: The original code uses both el-col with column spans and an inline div. It would be clearer to switch to using solely el-col with appropriate span settings.

  2. Improve Flexbox Structure: Use CSS Grid instead of combined Flexbox layouts where possible because it handles grid alignment more gracefully.

  3. Remove Extra Elements: The <div> around each row element is unnecessary, making the structure cleaner.

Here's how the improved version could look:

<div class="flex justify-start">
    <el-scrollbar height="525px" class="moz-height">
        <div class="h-app-card w-full" v-for="(app, index) in apps" :key="index">
            <div class="grid grid-cols-2 items-center space-x-2 grow">
                <div>
                    <el-avatar shape="square" :size="55" :src="'data:image/png;base64,' + app.icon" />
                </div>
                <div class="grow">
                    <div class="flex flex-col h-app-content">
                        <span class="h-app-title">{{ app.name }}</span>
                        <div class="h-app-desc">
                            <span>{{ language == 'zh' || language == 'tw' ? app.shortDescZh : app.shortDescEn }}</span>
                        </div>
                    </div>
                </div>
                <div>
                    <el-button class="h-app-button" type="primary" @click="installApp(app)">Install</el-button>
                </div>
            </div>
            <div class="h-app-divider" />
        </div>
    </el-scrollbar>
</div>

Key Improvements:

  • Use Grid Layout: This makes the layout more organized and easier to understand.
  • Simplify HTML Structure:Removed extra <div> elements for better readability and consistency.
  • Flex Properties: Used flex properties within .grow classes for dynamic sizing relative to surrounding content.

This approach maintains similar functionality while improving overall aesthetics and maintainability of the codebase.

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/toolbox/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div>
<RouterButton :buttons="buttons">
<template #route-button>
<div class="router-button">
<div class="router-button mr-0 sm:mr-7">
<el-button link type="primary" @click="onRestart('1panel')">
{{ $t('home.restart_1panel') }}
</el-button>
Expand Down
Loading