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(dropdown): [dropdown] add right side expansion function for dropdown #2608

Merged
merged 2 commits into from
Dec 11, 2024
+183 −10
Merged

feat(dropdown): [dropdown] add right side expansion function for dropdown #2608

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
62b2793
feat(dropdown): [dropdown] add right side expansion function for drop…
MomoPoppy Dec 4, 2024
64bdaaa
feat(dropdown): [dropdown] add right side expansion
MomoPoppy Dec 11, 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
7 changes: 5 additions & 2 deletions examples/sites/demos/apis/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,10 @@ export default {
'en-US': 'Menu pop-up location '
},
mode: ['pc'],
pcDemo: 'basic-usage'
pcDemo: 'placement',
meta: {
experimental: '3.21.0'
}
},
{
name: 'popper-class',
Expand Down Expand Up @@ -551,7 +554,7 @@ type IButtonType = 'default' | 'primary' | 'success' | 'warning' | 'danger' | 'i
name: 'IPlacementType',
type: 'type',
code: `
type IPlacementType = 'top' | 'top-start' | 'top-end' | 'bottom' | 'bottom-start' | 'bottom-end'
type IPlacementType = 'bottom-start' | 'bottom-end'
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Breaking Change Confirmed: IPlacementType restriction impacts existing code

The verification shows extensive usage of 'top' related placement values across the codebase, particularly in:

  • Multiple tooltip components using 'top', 'top-start', and 'top-end' placements
  • Popover components relying on top placements
  • Various UI components (Select, Grid, Tree) using top positioning

The removal of these options from IPlacementType will break existing implementations. Specific impacts:

  • 80+ instances of top-related placements found in component templates
  • Test files containing top placement assertions
  • Demo and documentation examples showcasing top placement functionality

Recommendations:

  1. Maintain these placement options in the type definition
  2. If removal is necessary, implement a deprecation cycle first
  3. Document migration path in release notes
🔗 Analysis chain

Breaking Change: IPlacementType restriction.

The removal of 'top', 'top-start', and 'top-end' options from IPlacementType is a breaking change that could affect existing implementations.

Let's verify the impact:

Consider:

  1. Adding a migration guide
  2. Deprecating the removed options first
  3. Maintaining backward compatibility until the next major version

Let me gather more information to verify the impact comprehensively.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of removed placement values
rg -l "placement.*['\"]top(?:-start|-end)?['\"]" --type vue

# Also check documentation
rg "placement.*top" examples/sites/demos/apis/dropdown.js

Length of output: 242

Script:

#!/bin/bash
# Search for usage of placement prop with top values in all files
rg -A 2 "placement['\"]?\s*[:=]\s*['\"]top" 

# Search for IPlacementType usage
rg "IPlacementType"

# Search for any documentation or examples mentioning placement
fd "README|CHANGELOG|docs" --type f --exec rg -l "placement"

Length of output: 41346

`
}
]
Expand Down
50 changes: 50 additions & 0 deletions examples/sites/demos/pc/app/dropdown/placement-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<template>
<tiny-dropdown>
<template #dropdown>
<tiny-dropdown-menu placement="bottom-start">
<tiny-dropdown-item label="老友粉"></tiny-dropdown-item>
<tiny-dropdown-item>黄金糕</tiny-dropdown-item>
<tiny-dropdown-item>狮子头</tiny-dropdown-item>
<tiny-dropdown-item>螺蛳粉</tiny-dropdown-item>
<tiny-dropdown-item disabled>双皮奶</tiny-dropdown-item>
<tiny-dropdown-item>蚵仔煎</tiny-dropdown-item>
</tiny-dropdown-menu>
</template>
</tiny-dropdown>
<br />
<br />
<tiny-dropdown>
<template #dropdown>
<tiny-dropdown-menu :options="options" placement="bottom-start"> </tiny-dropdown-menu>
</template>
</tiny-dropdown>
</template>
Comment on lines +1 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Demo should demonstrate right-side expansion functionality

The template only shows bottom-start placement without demonstrating the new right-side expansion feature that this PR introduces. Consider adding examples that showcase both left and right expansion behaviors.

Suggested additions:

 <tiny-dropdown>
   <template #dropdown>
-    <tiny-dropdown-menu placement="bottom-start">
+    <tiny-dropdown-menu placement="bottom-end">
       <tiny-dropdown-item label="老友粉"></tiny-dropdown-item>
       <!-- ... -->
     </tiny-dropdown-menu>
   </template>
 </tiny-dropdown>
+<!-- Add documentation comment explaining the different placement options -->
+<tiny-dropdown>
+  <template #dropdown>
+    <tiny-dropdown-menu placement="bottom-start" expand="right">
+      <!-- Add example with right expansion -->
+    </tiny-dropdown-menu>
+  </template>
+</tiny-dropdown>

Committable suggestion skipped: line range outside the PR's diff.


<script setup>
import { reactive } from 'vue'
import { iconStarDisable } from '@opentiny/vue-icon'
import { TinyDropdown, TinyDropdownMenu, TinyDropdownItem } from '@opentiny/vue'
const options = reactive([
{
label: '老友粉1',
icon: iconStarDisable(),
children: [
{
label: '老友粉2.1',
children: [{ label: '狮子头3.1' }]
},
{ label: '老友粉2.2' },
{ label: '老友粉2.3', disabled: true }
]
},
{
label: '狮子头',
disabled: true
},
{
label: '黄金糕',
icon: iconStarDisable()
}
])
</script>
60 changes: 60 additions & 0 deletions examples/sites/demos/pc/app/dropdown/placement.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<template>
<tiny-dropdown>
<template #dropdown>
<tiny-dropdown-menu placement="bottom-start">
<tiny-dropdown-item label="老友粉"></tiny-dropdown-item>
<tiny-dropdown-item>黄金糕</tiny-dropdown-item>
<tiny-dropdown-item>狮子头</tiny-dropdown-item>
<tiny-dropdown-item>螺蛳粉</tiny-dropdown-item>
<tiny-dropdown-item disabled>双皮奶</tiny-dropdown-item>
<tiny-dropdown-item>蚵仔煎</tiny-dropdown-item>
</tiny-dropdown-menu>
</template>
</tiny-dropdown>
<br />
<br />
<tiny-dropdown>
<template #dropdown>
<tiny-dropdown-menu :options="options" placement="bottom-start"> </tiny-dropdown-menu>
</template>
</tiny-dropdown>
</template>

<script>
import { iconStarDisable } from '@opentiny/vue-icon'
import { TinyDropdown, TinyDropdownMenu, TinyDropdownItem } from '@opentiny/vue'
export default {
components: {
TinyDropdown,
TinyDropdownMenu,
TinyDropdownItem
},
data() {
return {
options: [
{
label: '老友粉1',
icon: iconStarDisable(),
children: [
{
label: '老友粉2.1',
children: [{ label: '狮子头3.1' }]
},
{ label: '老友粉2.2' },
{ label: '老友粉2.3', disabled: true }
]
},
{
label: '狮子头',
disabled: true
},
{
label: '黄金糕',
icon: iconStarDisable()
}
]
}
}
}
</script>
14 changes: 14 additions & 0 deletions examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ export default {
},
codeFiles: ['disabled.vue']
},
{
demoId: 'placement',
name: {
'zh-CN': '展开位置',
'en-US': 'Placement'
},
desc: {
'zh-CN':
'<p>通过 <code>placement</code> 属性设置为 <code>bottom-start</code> 设置右侧展开。默认值为左侧展开。\n',
'en-US':
'<p>Set the <code>placement</code> attribute to <code>bottom-start</code> to expand on the right side. The default value is left expansion. </p>\n'
},
codeFiles: ['placement.vue']
zzcr marked this conversation as resolved.
Show resolved Hide resolved
},
{
demoId: 'size',
name: {
Expand Down
50 changes: 48 additions & 2 deletions packages/theme/src/dropdown-item/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
@import './vars.less';

@dropdown-item-prefix-cls: ~'@{css-prefix}dropdown-item';
@dropdown-menu-prefix-cls: ~'@{css-prefix}dropdown-menu';
@svg-prefix-cls: ~'@{css-prefix}svg';

.@{dropdown-item-prefix-cls} {
Expand Down Expand Up @@ -69,7 +70,7 @@
.@{dropdown-item-prefix-cls}__expand {
display: flex;
align-items: center;
margin-right: var(--tv-DropdownItem-expand-margin-right);
margin-right: var(--tv-DropdownItem-expand-margin-x);

& + .@{dropdown-item-prefix-cls}__content {
margin-left: 0;
Expand All @@ -88,7 +89,7 @@
}

.@{dropdown-item-prefix-cls}__pre-icon {
margin-right: var(--tv-DropdownItem-pre-icon-margin-right);
margin-right: var(--tv-DropdownItem-pre-icon-margin-x);
}
}
}
Expand Down Expand Up @@ -155,3 +156,48 @@
height: var(--tv-DropdownItem-height-xs);
}
}

/* 右侧展开 */
.@{dropdown-menu-prefix-cls}[x-placement='bottom-start'],
.@{dropdown-menu-prefix-cls}[x-placement='top-start'] {
&:has(.has-children) {
&,
.@{dropdown-menu-prefix-cls} {
& > li:not(.has-children) .@{dropdown-item-prefix-cls}__content {
margin-left: 0;
}
}
}

.@{dropdown-item-prefix-cls}__wrap {
flex-direction: row-reverse;

.@{dropdown-item-prefix-cls}__expand {
margin-right: 0;
margin-left: var(--tv-DropdownItem-expand-margin-x);

.tiny-svg {
transform: scaleX(-1);
}
}

.@{dropdown-item-prefix-cls}__content {
flex-direction: row-reverse;
justify-content: flex-end;

.@{dropdown-item-prefix-cls}__pre-icon {
margin-left: var(--tv-DropdownItem-pre-icon-margin-x);
margin-right: 0;
}

.@{dropdown-item-prefix-cls}__label {
margin-left: 0;
}
}

.@{dropdown-item-prefix-cls}--child {
right: unset;
left: 100%;
}
}
}
12 changes: 6 additions & 6 deletions packages/theme/src/dropdown-item/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,17 @@

// 菜单项内边距
--tv-DropdownItem-padding: 0px var(--tv-space-xl, 16px);
// 展开图标的右侧外边距
--tv-DropdownItem-expand-margin-right: var(--tv-space-md, 8px);
// 前置图标的右侧外边距
--tv-DropdownItem-pre-icon-margin-right: var(--tv-space-sm, 4px);
// 展开图标的水平外边距
--tv-DropdownItem-expand-margin-x: var(--tv-space-md, 8px);
// 自定义图标的水平外边距
--tv-DropdownItem-pre-icon-margin-x: var(--tv-space-sm, 4px);
Comment on lines +38 to +41
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add variables to support right-side expansion

While renaming margin variables to be more generic is good, additional variables are needed to fully support right-side expansion functionality.

Consider adding:

 // 展开图标的水平外边距
 --tv-DropdownItem-expand-margin-x: var(--tv-space-md, 8px);
 // 自定义图标的水平外边距
 --tv-DropdownItem-pre-icon-margin-x: var(--tv-space-sm, 4px);
+// 右侧展开时的位置调整
+--tv-DropdownItem-expand-right-offset: var(--tv-space-xl, 16px);
+// 右侧展开时的图标旋转
+--tv-DropdownItem-expand-right-rotation: 270deg;
📝 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-DropdownItem-expand-margin-x: var(--tv-space-md, 8px);
// 自定义图标的水平外边距
--tv-DropdownItem-pre-icon-margin-x: var(--tv-space-sm, 4px);
// 展开图标的水平外边距
--tv-DropdownItem-expand-margin-x: var(--tv-space-md, 8px);
// 自定义图标的水平外边距
--tv-DropdownItem-pre-icon-margin-x: var(--tv-space-sm, 4px);
// 右侧展开时的位置调整
--tv-DropdownItem-expand-right-offset: var(--tv-space-xl, 16px);
// 右侧展开时的图标旋转
--tv-DropdownItem-expand-right-rotation: 270deg;

// 菜单项内容区的左侧外边距
--tv-DropdownItem-content-margin-left: calc(
var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-expand-margin-right)
var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-expand-margin-x)
);
// 菜单项文本的左侧外边距
--tv-DropdownItem-label-margin-left: calc(
var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-pre-icon-margin-right)
var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-pre-icon-margin-x)
);
Comment on lines +44 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

Update margin calculations for right-side expansion

The current margin calculations need to be updated to handle both left and right expansion scenarios.

Consider updating the calculations:

 --tv-DropdownItem-content-margin-left: calc(
-  var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-expand-margin-x)
+  var(--tv-expand-direction) * (var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-expand-margin-x))
 );
 --tv-DropdownItem-label-margin-left: calc(
-  var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-pre-icon-margin-x)
+  var(--tv-expand-direction) * (var(--tv-DropdownItem-icon-size) + var(--tv-DropdownItem-pre-icon-margin-x))
 );

Committable suggestion skipped: line range outside the PR's diff.


// 菜单项高度(默认）
Expand Down
Loading