-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
perf: upload file count(#2029) #2061
Conversation
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -191,7 +221,7 @@ import { t } from '@/locales' | |||
const router = useRouter() | |||
const route = useRoute() | |||
const { | |||
query: { mode, question }, | |||
query: { mode, question } | |||
} = route as any | |||
const quickInputRef = ref() | |||
const props = withDefaults( |
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.
The HTML template you've provided contains some elements that need adjustment for better readability and responsiveness. Here are my suggestions:
-
Use Consistent Column Layout:
- The use of
el-row
andel-col
is consistent but lacks proper column widths for screens larger than desktop.
- The use of
-
Optimize Image Loading:
- Ensure images are loaded efficiently by setting a default width or using CSS to limit image size before loading them.
-
Code Formatting:
- There's minor indentation issues in some parts. Make sure code blocks are consistently indented if necessary.
-
Responsive Design Considerations:
- Use Bootstrap grid system classes (
col-*
) instead of custom styles like:xs
,:sm
, etc., for more flexibility.
- Use Bootstrap grid system classes (
This adjusted version should improve both the look and performance of the component:
@@ -2,6 +2,6 @@
<span>Upload Files ({uploadVideoList.length} >/ ${props.applicationDetails.file_upload_setting.maxFiles})</span>
">
<el-card shadow="never" style="--el-card-padding: 8px;" class="file cursor">...</el-card>,
- </span>
- </el-scrollbar>
+ </el-scrollbar></div>
<div class="flex">
Remember to review specific styling needs based on your design requirements, especially if you aim for responsiveness across different device sizes.
@@ -137,9 +163,6 @@ onMounted(() => {}) | |||
} | |||
} | |||
|
|||
.show { | |||
display: block; | |||
} | |||
|
|||
.download-button { | |||
display: none; |
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.
There are several issues and optimizations in the provided code snippet:
Issues:
- Redundant CSS Properties: The
width
property of.download-file
is redundant since Bootstrap's card component already sets default sizes. - Inline Styles in
<card>
: Inline styles like--el-card-padding
,cursor
, etc., should be managed through SCSS variables to maintain consistency. - Class Names: Consistent naming convention for classes like
download-button
can make it easier to manage and extend.
Optimization Suggestions:
- Use Bootstrap Components Efficiently: Instead of using custom inline styles, try to leverage Bootstrap cards' built-in features if possible.
- Consistent Class Naming: Name components similarly across different blocks of HTML for ease of maintaining.
- Reduce Redundancy: Remove unnecessary elements or repeat them where they are not needed.
Improved Code:
<script setup lang="ts">
// Your existing scripts here
</script>
<template>
<div class="content">
<div class="text break-all pre-wrap">
<div class="mb-8" v-if="document_list.length">
<el-row :gutter="10">
<el-col
v-for="(item, index) in document_list"
:key="index"
:xs="24"
:sm="12"
:md="12"
:lg="12"
:xl="12"
class="mb-8 w-full"
>
<el-card shadow="never" class="download-file cursor">
<div class="flex align-center download-button" @click="downloadFile(item)">
<!-- Custom icon goes here -->
<i class="fa fa-download"></i> Download File <!-- Add appropriate fontawesome icon -->
</div>
<div class="flex align-center show ml-4 el-ellipsis-1" title="File Name">
{{ item && item.name }}
</ div>
</el-card>
</el-col>
</el-row>
</div>
<div class="mb-8" v-if="image_list.length">
<el-row :gutter="10">
<el-col
v-for="(item, index) in image_list"
:key="index"
:xs="24"
:sm="12"
:md="12"
:lg="12"
:xl="12"
class="mb-8"
>
<template v-if="item.url">
<div class="file cursor border-r-4">
<el-image :src="item.url" lazy fit="cover" width="150px"></el-image>
</div>
</template>
</el-col>
</el-row>
</div>
<div class="mb-8" v-if="audio_list.length">
<el-row :gutter="10">
<el-col
v-for="(item, index) in audio_list"
:key="index"
:xs="24"
:sm="12"
:md="12"
:lg="12"
:xl="12"
class="mb-8"
>
<template v-if="item.url">
<div class="file cursor border-r-4">
<audio controls :src="item.url" preload></audio>
</div>
</template>
</el-col>
</el-row>
</div>
{{ chatRecord.problem_text }}
</div>
<!-- Other components remain unchanged -->
</div>
</template>
<style lang="scss" scoped>
.download-file {
padding: 8px; /* Set consistent padding */
}
.download-button,
.show {
cursor: pointer;
}
</style>
This version removes duplicate properties, improves use of Bootstrap components, maintains a more structured layout with rows and columns, and uses consistent class names across similar structures for better maintainability.
What this PR does / why we need it?
Summary of your change
Please indicate you've done the following: