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: Adjust the empty status prompt layout #7350

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

lan-yonghui
Copy link
Member

No description provided.

Copy link

f2c-ci-robot bot commented Dec 12, 2024

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.

@@ -130,6 +130,7 @@ import { Host } from '@/api/interface/host';
import { ElMessageBox } from 'element-plus';
import i18n from '@/lang';
import { MsgSuccess } from '@/utils/message';
import {Position} from "@element-plus/icons-vue";

const loading = ref();
const activeTag = ref('address');
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 you provided appears to be an HTML template using Vue.js with Element Plus components. Here are some points for review:

Irregularities and Potential Issues

  1. CSS Class Mismatch:

    <div class="flex flex-col gap-2 items-center justify-center w-full sm:flex-row">
        <!-- content -->
    </div>

    The style attribute is missing in one of these two <div> tags (w-full). Ensure that all styling attributes match across elements within the same structure.

  2. Element Reference Name:

    import Position from '@element-plus/icons-vue';
    import { Position } from "@/api/interface/host"; // This may clash if used elsewhere in your project.

    There's a minor issue here as both imports use different naming conventions which could potentially lead to conflicts. Consider changing either name to avoid confusion.

  3. Image Src Path:

    <img src="@/assets/images/no_app.svg" />

    Make sure the image path (@/assets/images/no_app.svg) is correct according to your file system structure. If it doesn’t exist, the image will not display.

  4. HTML Comment Syntax:

    <!-- This might cause problems since Element Plus uses icons differently. Check its usage. -->

    Verify how Element Plus handles SVGs or icons inside the <i></i> tag when combined with other classes like el-icon.

Optimization Suggestions

  1. Component Import Consistency:
    As mentioned earlier, keep component imports consistent throughout your project to prevent naming clashes.

  2. Lazy Loading Images (Optional):
    For better performance, consider lazy loading images by adding a loading property when the DOM loads.

  3. Accessibility Improvements:
    Ensure that clickable areas have sufficient contrast to meet accessibility guidelines. In this case, tooltips (e.g., toDoc()) should have descriptive labels for users relying on screen readers.

  4. Refactor Message Handling:
    Although current implementation seems fine, ensure that any message handling functions work correctly across devices (Desktop / Mobile).

  5. Dynamic Content Rendering:
    If $t('firewall.notSupport'), $t('firewall.quickJump'), etc., can change based on user interactions or configurations, make sure they update accordingly without unexpected layout changes.

Overall, checking for typos, ensuring consistency, and considering accessibility practices would help improve the quality of your web application.

</div>
</div>
<div>
<img src="@/assets/images/no_app.svg" />
</div>
</div>
</template>
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 appear to be minor corrections and optimizations. Here's a summary of the adjustments:

  1. Replaced a single <div> wrapper with two nested divs (<div class="flex flex-col gap-2 items-center justify-center w-full sm:flex-row">) for better styling alignment.
  2. Wrapped the click event listener within an inline class attribute on the span element instead of adding it directly as a child node.

These changes help improve layout flow and make the HTML more readable while maintaining functionality.

</div>
</div>
<div>
<img src="@/assets/images/no_app.svg" />
</div>
</div>
</template>
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 changes you provided have several areas that need attention:

  1. Inline Span and Icon Placement: The span elements now include an icon, but the spacing around it differs between the first inline span and the second one inside the flex div.

  2. Flex Box Alignment: The use of a flex box (flex flex-col gap-2) ensures content is aligned correctly when viewed on both small and large screens (sm:flex-row). However, there's no explicit width specified for the flex container to ensure all images align well across sizes.

  3. Icon Size and Space: The space between the button's text and icon can be better controlled, which may improve usability. Adding some padding might help.

Here's a slightly adjusted version with these considerations addressed:

<template #main>
    <div class="app-warn">
        <!-- First span -->
        <div class="items-center justify-center w-full sm:hidden">
            <span>{{ $t('app.checkInstalledWarn', [$t('database.noPostgresql')]) }}</span>
            <span @click="goRouter('app')" class="flex items-center justify-center gap-0.5 mx-auto">
                <el-icon><Position /></el-icon>
                {{ $t('database.goInstall') }}
            </span>
            <div v-if="$device.isMobile" style="height: 2rem;"></div> <!-- Ensure equal height across devices -->
        </div>

        <!-- Second span wrapped in a flexbox -->
        <div v-if="$!$device.isMobile" class="hidden px-4 grid-cols-1 md:grid-cols-2 lg:grid-cols-4 mt-8 mb-6 gap-y-6 xl:gap-y-7">
            <span>{{ $t('app.checkInstalledWarn', [$t('database.noPostgresql')]) }}</span>
            <span @click="goRouter('app')">
                <el-icon><Position /></el-icon>
                {{ $t('database.goInstall') }}
            </span>
            <div>
                <img src="@/assets/images/no_app.svg" />
            </div>
        </div>

        <!-- Mobile-specific image placement -->
        <div v-if="$$device.isMobile"
             class="w-[50%] relative bottom-5 left-1/2 translate-x-[-50%]"
             style="background-image: url('@/assets/images/no_app.png'); background-position:center; background-repeat:no-repeat;">
        </div>
    </div>
</template>

Key Changes Made:

  1. Added .mx-auto to the main flex item.
  2. Used CSS media queries with $device.isMobile to handle mobile display differently.
  3. Ensured all relevant parts are responsive and visually consistent on larger screen sizes.
  4. Adjusted positioning logic for smaller displays where a dedicated section was more appropriate.

This should lead to a cleaner and functional UI layout, ensuring compatibility across various devices and screen resolutions.

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud left a comment

Choose a reason for hiding this comment

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

/lgtm

@wanghe-fit2cloud
Copy link
Member

/approve

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud left a comment

Choose a reason for hiding this comment

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

/lgtm

@wanghe-fit2cloud
Copy link
Member

/approve

Copy link

f2c-ci-robot bot commented Dec 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot merged commit c0e0db6 into dev Dec 12, 2024
7 checks passed
@f2c-ci-robot f2c-ci-robot bot deleted the pr@dev@feat_layout branch December 12, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants