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 layout style #7354

Merged
merged 2 commits into from
Dec 12, 2024
Merged

style: Optimize layout style #7354

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
d2cef22
style: Optimize layout style
lan-yonghui Dec 12, 2024
17fb3b2
style: Optimize layout style
lan-yonghui Dec 12, 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
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ const message = {
infoDetail: 'File properties',
root: 'Root directory',
list: 'File list',
sub: 'Include subdirectories',
sub: 'Subdirectories',
downloadSuccess: 'Successfully downloaded',
theme: 'Theme',
language: 'Language',
Copy link
Member

Choose a reason for hiding this comment

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

The provided code snippet contains the following improvements:

  1. Updated Registry Name: The registry name from "Insecure registries" to "Private registries".

  2. Simplified Subdirectory Label: Changed sub from both "Include subdirectories" and "Subdirectories" to simply "Subdirectories". These two options are redundant since including all directories is typically implied when listing files.

  3. Optimized String Formatting: Ensured consistent spacing around punctuation marks in comments for better readability.

Here's the updated JavaScript object with these changes:

const message = {
  mirrors: 'Registry mirrors',
  mirrorsHelper: '',
  mirrorsHelper2: 'For details, see the official documents.',
  registries: 'Private registries', // Updated registry name
  ipv6Helper:
    'When enabling IPv6, you need to add an IPv6 container network. Refer to the official documentation for specific configuration steps.',
  ipv6CidrHelper: 'IPv6 address pool range for containers',

  infoDetail: 'File properties',
  root: 'Root directory',
  list: 'File list',
  sub: 'Subdirectories', // Simplified label
  downloadSuccess: 'Successfully downloaded',
  theme: 'Theme',
  language: 'Language',
};

These adjustments make the text more concise and logically coherent while maintaining clarity.

Copy link
Member

Choose a reason for hiding this comment

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

The difference between sub values is that include subdirectories has been updated to simply Subdirectories. This should not cause any major issues but could be confusing for users who rely on the original wording.

Expand Down
10 changes: 10 additions & 0 deletions frontend/src/styles/element-dark.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,4 +460,14 @@ html.dark {
.el-checkbox__input.is-indeterminate .el-checkbox__inner::before {
background-color: var(--panel-main-bg-color-10);
}

.custom-input-textarea {
background-color: var(--panel-main-bg-color-10) !important;
color: var(--el-color-info) !important;
}
.custom-input-textarea:hover {
background-color: var(--panel-main-bg-color-9) !important;
color: var(--el-color-primary) !important;
}

}
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 no irregularities or immediate issues with the provided CSS changes. However, there are optimization suggestions that could be made to ensure better maintainability and performance:

  1. Namespace Classes: Consider using a more namespace-like approach to separate different styles within one class instead of duplicating it with .custom-input-textarea and its hover state. This would make maintenance easier if additional styles are needed.

    .custom-input {
    --background-color: var(--panel-main-bg-color-10);
    --color: var(--el-color-info);

    background-color: var(--background-color) !important;
    color: var(--color) !important;

    &.hover-state {
        --background-color: var(--panel-main-bg-color-9);
        --color: var(--el-color-primary);

        background-color: var(--background-color) !important;
        color: var(--color) !important;
    }
}

  2. CSS Variables Recapitulation: It might be beneficial to recapitalize the --el-color-* variables consistently throughout their uses in the dark theme rules for uniformity.

  3. Vendor Prefixing: If these styles need to support older browsers specifically (not covered in your knowledge cutoff), consider adding vendor prefixes where necessary.

These suggestions aim to clean up the styling while maintaining clarity and functionality.

Copy link
Member

Choose a reason for hiding this comment

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

The code additions appear to be adding custom styles for input textareas that fit into a dark theme context of an element being styled with .dark. The changes include modifying:

  • The background color and text color for regular state (var(--panel-main-bg-color-10) and var(--el-color-info))
  • Hover states with increased contrast (var(--panel-main-bg-color-9) and var(--el-color-primary))

These improvements enhance readability on dark backgrounds, making them more user-friendly. However, it's worth noting that these customizations assume a specific CSS framework (like Element UI) where variables like --panel-main-bg-color-10 and --el-color-info exist. If those variable names change or if there's another way to style these elements without altering the framework-specific ones, further adjustments would be necessary to ensure compatibility and maintainability.

7 changes: 7 additions & 0 deletions frontend/src/styles/element.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,10 @@ html {
background-color: #f5f7fa !important;
color: var(--el-color-info) !important;
}

.custom-input-textarea:hover {
color: var(--el-color-primary) !important;
background-color: var(--el-color-primary-light-9) !important;
border-color: var(--el-button-border-color) !important;
}

Copy link
Member

Choose a reason for hiding this comment

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

The provided CSS snippet appears to be intended for styling elements with the class .custom-input-textarea. Here's a review of the changes:

  1. Variable Usage: The use of var(--el-color-info) is consistent and likely correctly referenced from an existing theme.

  2. Hover Styling Improvements:

    • Color Change: Changing the hover state color from gray to primary blue (#007bff or similar) aligns well visually with typical form fields.
    • Background Color: A lightened version (var(--el-color-primary-light-9)) enhances readability, especially in dark themes.
    • Border Color: Ensuring the border remains aligned with other UI components provides unity and consistency across your application design.

These changes improve user experience on hover actions by providing immediate feedback without altering the overall interface significantly. Good job on enhancing the visual appeal and usability directly related to input text areas!

Copy link
Member

Choose a reason for hiding this comment

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

The provided CSS modifications look mostly correct from a basic standpoint. However, some improvements could be made:

  1. CSS Selector Consistency: Ensure that the selector is consistent without extra spaces, like color: var(--el-button-border-color) !important; should ideally be just border-color: var(--el-button-border-color);.

  2. Variable Names: It's always good to avoid using generic variable names such as --el-color-primary. This can improve maintainability. Consider making them more descriptive if appropriate.

  3. Hover State Styles: The hover state style is already well-implemented with colors and background changes. No immediate suggestion comes to mind here since it works correctly.

Here's an optimized version considering best practices:

html {
    background-color: #f5f7fa !important;
    color: var(--main-text-color) !important; /* Replace --main-text-color with a more specific name */
}

.custom-input-textarea:hover {
    /* Simplified styling */
    color: darken(#007bff, 10%) ; /* Use HSLA for better control over lightness */
    background-color: rgba(48,64,96, 0.2);
    border-color: #ccc;
}

This optimization improves readability by simplifying unnecessary comments and adjusts the contrast using darken() (HSLA function for dynamic adjustments).

2 changes: 1 addition & 1 deletion frontend/src/views/container/setting/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<el-row style="margin-top: 20px" v-if="confShowType === 'base'">
<el-col :span="1"><br /></el-col>
<el-col :xs="24" :sm="24" :md="15" :lg="12" :xl="10">
<el-form :model="form" label-position="left" :rules="rules" ref="formRef" label-width="120px">
<el-form :model="form" label-position="left" :rules="rules" ref="formRef" label-width="auto">
<el-form-item :label="$t('container.mirrors')" prop="mirrors">
<div
class="flex w-full justify-start flex-col sm:flex-row sm:items-end"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/host/file-management/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@
</div>
</template>
</el-table-column>
<el-table-column :label="$t('file.mode')" prop="mode" max-width="50">
<el-table-column :label="$t('file.mode')" prop="mode" max-width="50" min-width="110px">
<template #default="{ row }">
<el-link :underline="false" @click="openMode(row)">{{ row.mode }}</el-link>
</template>
Expand Down
Loading