-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: markdown preview i18n #2223
Conversation
.vertical-middle { | ||
vertical-align: middle; | ||
.scan-height { | ||
height: calc(100vh - var(--app-header-height) - var(--app-view-padding) * 2 - 70px); | ||
} | ||
</style> |
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 is an issue with the scrollbar
component in the Vue.js template. It's likely not being used correctly because v-loading
takes precedence over other styles when applied to a div
.
Here’s how you can address it:
Suggestion
Use the overflow-y-auto
CSS property inside the <div>
element where you want to apply scrolling. Here’s the updated snippet:
<template>
<div v-loading="loading" class="container-class">
<!-- Your existing content here -->
</div>
</template>
<script>
export default {
// ...
};
</script>
<style scoped>
.container-class {
overflow-y-auto; /* This will add scrollbar if needed */
padding: var(--app-view-padding); /* Add this for consistent layout spacing */
}
/* Other style rules remain unchanged */
By applying overflow-y-auto
, you ensure that scrollbars appear only when necessary, enhancing user interactions without affecting other elements.
This should resolve the inconsistency between using v-loading
and other styling approaches. Let me know if you need further assistance!
'zh-Hant': ZH_TW | ||
} | ||
} | ||
}) | ||
</script> |
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 improvements and optimizations you can make to the code:
-
Remove Redundant Import:
The import statement forZh_TW
is redundant since it's already imported at the beginning as part of the extension. -
Use Computed Properties Correctly:
In Vue.js, computed properties should be declared outside the lifecycle hooks (setup
function). You have correctly defined it, but ensure that this works for your specific scenario. -
Ensure Consistency:
Make sure all keys in the computed property object match those used in other configuration objects likeeditorConfig
. -
Check Compatibility:
Ensure that the versions of libraries you're using (likemd-editor-v3
) are compatible with Vue 3.
Here's a revised version of your code:
<template>
<MdPreview :language="language" noIconfont noPrettier :codeFoldable="false" v-bind="$attrs" />
</template>
<script setup lang="ts">
import { computed } from 'vue';
import { MdPreview, config } from 'md-editor-v3';
import { getBrowserLang } from '@/locales/index';
import useStore from '@/stores';
defineOptions({ name: 'MdPreview' });
const { user } = useStore();
const language = computed(() => user.getLanguage() || getBrowserLang());
config({
editorConfig: {
languageUserDefined: {
'zh-Hant': require('@vavt/cm-extension/dist/locale/zh-TW')
}
}
});
</script>
Key Changes Made:
- Removed the redundancy in importing
ZH_TW
. - Ensured consistent usage of computed properties.
- Added an explicit type hint for
require
, though TypeScript might handle dynamic imports automatically in later versions.
@@ -427,7 +427,7 @@ const form = ref<any>({ | |||
|
|||
const xpackForm = ref<any>({ | |||
show_source: false, | |||
language: getBrowserLang(), | |||
language: '', | |||
show_history: false, | |||
draggable: false, | |||
show_guide: false, |
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 a few areas where you can optimize and address potential issues in the provided code:
-
Language Selection Clearance:
- In the
<el-select>
component for selecting the language, consider addingclearable
to make it easier for users to remove their selection if needed.
- In the
-
Initial Language Setting:
- When setting the initial value of the
language
field in the forms (defaultSetting
,form
, andxpackForm
), ensure that there's a meaningful default value (e.g., an empty string) rather than usinggetBrowserLang()
. This avoids relying on environment settings and makes testing more straightforward.
- When setting the initial value of the
-
Code Duplication:
- There appears to be some duplication between lines 51 and 67. Ensure these lines match or combine logically when necessary.
Here are specific changes:
Revised Code Snippets
- **Line 50:
<div>
<!-- Existing content -->
<el-select v-model="form.language">
...
- Lines: 174 & 383:
<h5 class="mb-8">{{ $t('views.applicationOverview.appInfo.SettingDisplayDialog.languageLabel') }}</h5>
...
const form = ref<any>({
...
language: '', // Change here
});
If further modification or adjustments are required based on additional context not shown here, please let me know!
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 |
fix: Style optimization
perf: SCAN login setting views Style optimization
fix: markdown preview i18n