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

feat(locales): [locales] Optimize the style and demo #2475

Closed
wants to merge 3 commits into from
Closed

feat(locales): [locales] Optimize the style and demo #2475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6f512f
fix(drawer): [drawer] modify problem (#2460)
James-9696 Oct 29, 2024
dd6e2c5
feat(locales): [locales] Optimize the style and demo
Youyou-smiles Oct 31, 2024
8acf1c7
feat(locales): [locales] Optimize style
Youyou-smiles Oct 31, 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
6 changes: 6 additions & 0 deletions examples/sites/demos/pc/app/amount/size-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<template>
<tiny-amount size="medium" v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount size="small" v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount size="mini" v-model="value" :date="date"></tiny-amount>
</template>

<script setup lang="jsx">
Expand Down
6 changes: 6 additions & 0 deletions examples/sites/demos/pc/app/amount/size.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<template>
<tiny-amount size="medium" v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount size="small" v-model="value" :date="date"></tiny-amount>
<br />
<tiny-amount size="mini" v-model="value" :date="date"></tiny-amount>
</template>

<script lang="jsx">
Expand Down
4 changes: 2 additions & 2 deletions examples/sites/demos/pc/app/area/custom-service-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function getJCR() {
id: '3623',
id_and_type: '3623JCR',
parent_and_type: null,
name: 'A公司 Marketing与销售服务部',
name: 'A公司销售服务部',
date_from: '2005/06/21 00:00:00',
name_en: 'Consumer BG Marketing and Sales Service Dept',
parent_type: null,
Expand All @@ -36,7 +36,7 @@ function getJCR() {
id: '650561',
id_and_type: '650561JCR',
parent_and_type: null,
name: 'A公司 Marketing与销售服务部（东莞）',
name: 'A公司销售服务部（东莞）',
date_from: '2012/09/27 00:00:00',
name_en: 'Consumer BG Marketing and Sales Service Dept (Dongguan)',
parent_type: null,
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/area/custom-service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test('测试自定义服务', async ({ page }) => {
await getSelectItem(areaRep).first().click()
await areaInput.nth(3).click()
await getSelectItem(areaOffice).first().click()
await expect(areaInput.first()).toHaveValue('A公司 Marketing与销售服务部')
await expect(areaInput.first()).toHaveValue('A公司销售服务部')
await expect(areaInput.nth(1)).toHaveValue('火星终端子公司')
await expect(areaInput.nth(2)).toHaveValue('北京代表处')
await expect(areaInput.nth(3)).toHaveValue('牙买加办事处')
Expand Down
4 changes: 2 additions & 2 deletions examples/sites/demos/pc/app/area/custom-service.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default {
id: '3623',
id_and_type: '3623JCR',
parent_and_type: null,
name: 'A公司 Marketing与销售服务部',
name: 'A公司销售服务部',
date_from: '2005/06/21 00:00:00',
name_en: 'Consumer BG Marketing and Sales Service Dept',
parent_type: null,
Expand All @@ -43,7 +43,7 @@ export default {
id: '650561',
id_and_type: '650561JCR',
parent_and_type: null,
name: 'A公司 Marketing与销售服务部（东莞）',
name: 'A公司销售服务部（东莞）',
date_from: '2012/09/27 00:00:00',
name_en: 'Consumer BG Marketing and Sales Service Dept (Dongguan)',
parent_type: null,
Expand Down
19 changes: 15 additions & 4 deletions examples/sites/demos/pc/app/drawer/use-through-method-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<script lang="tsx" setup>
import { ref } from 'vue'
import { TinyDrawer, TinyButton } from '@opentiny/vue'
import { TinyDrawer, TinyButton, Notify } from '@opentiny/vue'
import { iconHelp } from '@opentiny/vue-icon'

let drawerInstance = null
Expand All @@ -24,15 +24,26 @@ const config = ref({
showFooter: true,
// 通过 events 监听事件
events: {
open: (instance) => console.log('open 事件', instance),
close: () => console.log('close 事件')
open: (instance) =>
Notify({
type: 'info',
title: 'open 事件',
message: `${instance.title}`,
position: 'top-right'
}),
close: () =>
Notify({
type: 'info',
title: 'close 事件',
position: 'top-right'
})
},
// 通过属性 customSlots 设置插槽, 插槽内容可以是 string | VNode | ({h, $drawer}) => VNode
customSlots: {
// 使用 h 函数
default: (h) => h('p', { class: '' }, '抽屉主体内容。'),
// 返回 VNode 节点的方法, 可通过参数中 $drawer 访问到组件实例
headerRight: () => <IconHelp></IconHelp>,
headerRight: () => <IconHelp style="width:20px;height:20px"></IconHelp>,
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider moving styles to CSS classes.

While inline styles work, moving them to CSS classes would improve maintainability and reusability.

Consider this approach:

-    headerRight: () => <IconHelp style="width:20px;height:20px"></IconHelp>,
+    headerRight: () => <IconHelp class="drawer-header-icon"></IconHelp>,

And add to your component's style section:

<style>
.drawer-header-icon {
  width: 20px;
  height: 20px;
}
</style>

// 直接赋值 VNode
footer: (
<TinyButton type="primary" onClick={closeDrawer}>
Expand Down
29 changes: 20 additions & 9 deletions examples/sites/demos/pc/app/drawer/use-through-method.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</template>

<script lang="tsx">
import { TinyDrawer, TinyButton } from '@opentiny/vue'
import { TinyDrawer, TinyButton, Notify } from '@opentiny/vue'
import { iconHelp } from '@opentiny/vue-icon'

export default {
Expand All @@ -18,6 +18,9 @@ export default {
}
},
methods: {
closeDrawer() {
this.drawerInstance.close()
},
showDrawer() {
const IconHelp = iconHelp()
const config = {
Expand All @@ -27,27 +30,35 @@ export default {
showFooter: true,
// 通过 events 监听事件
events: {
open: (instance) => console.log('open 事件', instance),
close: () => console.log('close 事件')
open: (instance) =>
Notify({
type: 'info',
title: 'open 事件',
message: `${instance.title}`,
position: 'top-right'
}),
close: () =>
Notify({
type: 'info',
title: 'close 事件',
position: 'top-right'
})
},
// 通过属性 customSlots 设置插槽, 插槽内容可以是 string | VNode | ({h, $drawer}) => VNode
customSlots: {
// 使用 h 函数
default: (h) => h('p', { class: '' }, '抽屉主体内容。'),
// 返回 VNode 节点的方法, 可通过参数中 $drawer 访问到组件实例
headerRight: () => <IconHelp></IconHelp>,
headerRight: () => <IconHelp style="width:20px;height:20px"></IconHelp>,
// 直接赋值 VNode
footer: (
<Button type="primary" onClick={this.closeDrawer}>
<TinyButton type="primary" onClick={this.closeDrawer}>
点击关闭
</Button>
</TinyButton>
Comment on lines +52 to +57
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider moving inline styles to CSS classes

While the inline styles work, consider moving them to CSS classes for better maintainability and reusability.

-headerRight: () => <IconHelp style="width:20px;height:20px"></IconHelp>,
+headerRight: () => <IconHelp class="drawer-header-icon"></IconHelp>,

Add to your style section:

<style>
.drawer-header-icon {
  width: 20px;
  height: 20px;
}
</style>

)
}
}
this.drawerInstance = TinyDrawer.service(config)
},
closeDrawer() {
this.drawerInstance.close()
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions examples/sites/demos/pc/app/locales/custom-service-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import { TinyLocales } from '@opentiny/vue'

function getLocale() {
// resolve 出来的必须是国际化的 key
return Promise.resolve(['zhCN', 'enUS', 'zhTW'])
return Promise.resolve(['zh_CN', 'en_US', 'zh_TW'])
}

function getCurrentLocale() {
return Promise.resolve(['zhCN'])
return Promise.resolve(['zh_CN'])
}

function getChangeLocaleUrl(targetLocale) {
if (targetLocale === 'enUS') {
if (targetLocale === 'en_US') {
return Promise.resolve(`${window.location.origin}/#/webenglish/en-US/component/locales/custom-service`)
} else {
return Promise.resolve(`${window.location.origin}/#/zh-CN/component/custom-service`)
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/locales/custom-service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ test('locales-custom-service', async ({ page }) => {
const reference = demo.locator('.reference-wrapper')
const popper = page.locator('.tiny-locales__popper')

await expect(reference).toHaveText('zhCN')
await expect(reference).toHaveText('zh_CN')
Youyou-smiles marked this conversation as resolved.
Show resolved Hide resolved
await expect(popper).toBeHidden()
await reference.hover()
await page.waitForTimeout(200)
Expand Down
6 changes: 3 additions & 3 deletions examples/sites/demos/pc/app/locales/custom-service.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ export default {
methods: {
getLocale() {
// resolve 出来的必须是国际化的 key
return Promise.resolve(['zhCN', 'enUS', 'zhTW'])
return Promise.resolve(['zh_CN', 'en_US', 'zh_TW'])
},
getCurrentLocale() {
return Promise.resolve(['zhCN'])
return Promise.resolve(['zh_CN'])
},
getChangeLocaleUrl(targetLocale) {
if (targetLocale === 'enUS') {
if (targetLocale === 'en_US') {
return Promise.resolve(`${window.location.origin}/#/webenglish/en-US/component/locales/custom-service`)
} else {
return Promise.resolve(`${window.location.origin}/#/zh-CN/component/custom-service`)
Expand Down
9 changes: 6 additions & 3 deletions packages/theme/src/area/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@
*/

@import '../custom.less';
@import './vars.less';

@area-prefix-cls: ~'@{css-prefix}area';
@select-prefix-cls: ~'@{css-prefix}select';

.@{area-prefix-cls} {
.@{select-prefix-cls} {
width: 270px;
margin-right: 10px;
margin-bottom: 10px;
.inject-Area-vars();

width: 220px;
margin-right: var(--tv-Area-select-margin-right);
margin-bottom: var(--tv-Area-select-margin-bottom);;
}
}
17 changes: 17 additions & 0 deletions packages/theme/src/area/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2022 - present TinyVue Authors.
* Copyright (c) 2022 - present Huawei Cloud Computing Technologies Co., Ltd.
*
* Use of this source code is governed by an MIT-style license.
*
* THE OPEN SOURCE SOFTWARE IN THIS PRODUCT IS DISTRIBUTED IN THE HOPE THAT IT WILL BE USEFUL,
* BUT WITHOUT ANY WARRANTY, WITHOUT EVEN THE IMPLIED WARRANTY OF MERCHANTABILITY OR FITNESS FOR
* A PARTICULAR PURPOSE. SEE THE APPLICABLE LICENSES FOR MORE DETAILS.
*
*/

.inject-Area-vars() {
// 选中框间距
--tv-Area-select-margin-right: var(--tv-space-xl, 16px);
--tv-Area-select-margin-bottom: var(--tv-space-xl, 16px);
}
32 changes: 26 additions & 6 deletions packages/theme/src/locales/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,41 @@
*/

@import '../custom.less';
@import './vars.less';

@locales-prefix-cls: ~'@{css-prefix}locales';
@popover-prefix-cls: ~'@{css-prefix}popover';
@popper-prefix-cls: ~'@{css-prefix}popper';

.@{locales-prefix-cls} {
.inject-Locales-vars() ;

cursor: pointer;

svg {
margin-left: 2px;
margin-left: var(--tv-Locales-svg-margin-left);
}
&__text {
font-size: var(--tv-Locales-text-font-size);
color: var(--tv-Locales-svg-color);
display: inline-flex;
align-items: center;
}
&__icon {
display: inline-flex;
align-items: center;
justify-content: center;
svg {
width: var(--tv-Locales-svg-size);
height: var(--tv-Locales-svg-size);;
fill: var(--tv-Locales-text-color);
}
Comment on lines +34 to +42
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix double semicolon in height property

There's a syntax error in the SVG height property.

-      height: var(--tv-Locales-svg-size);;
+      height: var(--tv-Locales-svg-size);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&__icon {
display: inline-flex;
align-items: center;
justify-content: center;
svg {
width: var(--tv-Locales-svg-size);
height: var(--tv-Locales-svg-size);;
fill: var(--tv-Locales-text-color);
}
&__icon {
display: inline-flex;
align-items: center;
justify-content: center;
svg {
width: var(--tv-Locales-svg-size);
height: var(--tv-Locales-svg-size);
fill: var(--tv-Locales-text-color);
}

}

&__popper {
.inject-Locales-vars() ;
&.@{popover-prefix-cls}.@{popper-prefix-cls} {
padding: 0;
padding: var(--tv-Locales-popper-padding-h) 0;
}

.poplist {
Expand All @@ -34,11 +54,11 @@
li {
list-style: none;
cursor: pointer;
padding: 0 8px;
line-height: 30px;
padding: 0 var(--tv-Locales-popper-li-padding-h);
line-height: var(--tv-Locales-popper-li-line-heigaht);
Comment on lines +57 to +58
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in line-height variable name

There's a typo in the line-height variable name that could cause styling issues.

-        line-height: var(--tv-Locales-popper-li-line-heigaht);
+        line-height: var(--tv-Locales-popper-li-line-height);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
padding: 0 var(--tv-Locales-popper-li-padding-h);
line-height: var(--tv-Locales-popper-li-line-heigaht);
padding: 0 var(--tv-Locales-popper-li-padding-h);
line-height: var(--tv-Locales-popper-li-line-height);


&.selected {
background: #f5f5f5;
&:hover {
background-color: var(--tv-Locales-popper-li-hover-color);
}
}
}
Expand Down
32 changes: 32 additions & 0 deletions packages/theme/src/locales/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright (c) 2022 - present TinyVue Authors.
* Copyright (c) 2022 - present Huawei Cloud Computing Technologies Co., Ltd.
*
* Use of this source code is governed by an MIT-style license.
*
* THE OPEN SOURCE SOFTWARE IN THIS PRODUCT IS DISTRIBUTED IN THE HOPE THAT IT WILL BE USEFUL,
* BUT WITHOUT ANY WARRANTY, WITHOUT EVEN THE IMPLIED WARRANTY OF MERCHANTABILITY OR FITNESS FOR
* A PARTICULAR PURPOSE. SEE THE APPLICABLE LICENSES FOR MORE DETAILS.
*
*/

.inject-Locales-vars() {
// 图标距离文字间距
--tv-Locales-svg-margin-left: var(--tv-space-sm, 4px);
// 图标尺寸
--tv-Locales-svg-size: var(--tv-font-size-md, 14px);
// 图标颜色
--tv-Locales-svg-color: var(--tv-color-text, #191919);
// 文字字号
--tv-Locales-text-font-size: var(--tv-font-size-md, 14px);
// 文字颜色
--tv-Locales-text-color: var(--tv-color-text, #191919);
// 弹框间距
--tv-Locales-popper-padding-h: var(--tv-space-md, 8px);
// 弹框选项间距
--tv-Locales-popper-li-padding-h: var(--tv-space-xl, 16px);
// 弹框选项行高
--tv-Locales-popper-li-line-heigaht: 30px;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in CSS property name.

The property name contains a typo: "line-heigaht" should be "line-height".

-  --tv-Locales-popper-li-line-heigaht: 30px;
+  --tv-Locales-popper-li-line-height: 30px;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--tv-Locales-popper-li-line-heigaht: 30px;
--tv-Locales-popper-li-line-height: 30px;

// 弹框选项悬浮色
--tv-Locales-popper-li-hover-color: var(--tv-color-bg-hover, #f5f5f5);
}
2 changes: 1 addition & 1 deletion packages/vue/src/locales/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@opentiny/vue-locales",
"version": "3.19.0",
"version": "3.19.1",
"description": "",
"main": "lib/index.js",
"module": "index.ts",
Expand Down
8 changes: 4 additions & 4 deletions packages/vue/src/locales/src/pc.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
:append-to-body="popperAppendToBody"
>
<template #reference>
<span
<span class="tiny-locales__text"
>{{ state.text && t(state.text)
}}<span>
}}<span class="tiny-locales__icon">
<IconChevronDown v-show="!state.visible" />
<IconChevronUp v-show="state.visible" /> </span
></span>
Expand All @@ -39,13 +39,13 @@ import { $prefix, setup, defineComponent } from '@opentiny/vue-common'
import { renderless, api } from '@opentiny/vue-renderless/locales/vue'
import { use, enUS, zhCN } from '@opentiny/vue-locale'
import TinyPopover from '@opentiny/vue-popover'
import { IconChevronUp, IconChevronDown } from '@opentiny/vue-icon'
import { IconUpWard, IconChevronDown } from '@opentiny/vue-icon'
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can generate the final response since I have enough information to verify the icon change impact:

The icon change from IconChevronUp to IconUpWard appears to be inconsistent with the codebase patterns

The search results reveal that:

  • IconChevronUp and IconChevronDown are consistently used together as a pair across multiple components (user-link, user-contact, time-line, roles, etc.)
  • The IconUpWard icon is rarely used in the codebase, with only one other usage in the cascader component
  • The current change breaks this established pattern in the locales component by pairing IconChevronDown with IconUpWard

This change could lead to visual inconsistency since:

  • The component uses both icons for toggling states (visible/not visible)
  • IconChevronDown and IconChevronUp are designed as a matching pair
  • IconUpWard may have different styling/dimensions than the chevron icons
🔗 Analysis chain

Verify the visual impact of replacing IconChevronUp with IconUpWard.

The icon change from IconChevronUp to IconUpWard might affect the visual consistency of the component. Please ensure that:

  1. The new icon maintains visual harmony with IconChevronDown
  2. The icon size and style match the design system

Let's check if these icons are used consistently across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the usage patterns of IconChevronUp and IconUpWard
# to ensure consistent icon usage across the codebase

echo "Checking IconChevronUp usage:"
rg "IconChevronUp" -A 2

echo -e "\nChecking IconUpWard usage:"
rg "IconUpWard" -A 2

echo -e "\nChecking if these icons are paired with IconChevronDown:"
rg "IconChevronDown" -A 2

Length of output: 35861


export default defineComponent({
name: $prefix + 'Locales',
components: {
TinyPopover,
IconChevronUp: IconChevronUp(),
IconChevronUp: IconUpWard(),
Copy link
Member

Choose a reason for hiding this comment

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

这里修改了模板，需要确认下是否对saas主题造成影响

IconChevronDown: IconChevronDown()
Comment on lines +48 to 49
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider renaming the component registration for clarity.

The component is imported as IconUpWard but registered as IconChevronUp, which could be confusing for maintenance. Consider updating the registration name to match the imported component name.

Apply this diff to maintain consistency:

-    IconChevronUp: IconUpWard(),
+    IconUpward: IconUpWard(),

Then update the template usage:

-            <IconChevronUp v-show="state.visible" />
+            <IconUpward v-show="state.visible" />

Committable suggestion was skipped due to low confidence.

},
props: {
Expand Down
Loading