Skip to content

refactor(split): [split] refactor split theme #2194

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

Merged
merged 1 commit into from
Sep 27, 2024
Merged

refactor(split): [split] refactor split theme #2194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/basic-usage-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.5)
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/split/basic-usage.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ test('基本用法', async ({ page }) => {
const afterMove = leftPanelWidth + 100 - triggerBtnWidth / 2
const { width: afterWidth } = await leftPanel.boundingBox()
// 小数点后一位相同即可
await expect(afterMove).toBeCloseTo(afterWidth, 1)
await expect(Math.ceil(afterMove)).toEqual(Math.ceil(afterWidth))
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider the assertion change for width comparison

The change from toBeCloseTo to toEqual with Math.ceil might make the test more brittle and less representative of the actual component behavior. Consider the following alternatives:

  1. Retain toBeCloseTo with a small tolerance:

    await expect(afterMove).toBeCloseTo(afterWidth, 1);

  2. If exact equality is required, add a comment explaining why:

    // Exact pixel equality is required for this component
await expect(Math.round(afterMove)).toEqual(Math.round(afterWidth));

  3. Use a custom matcher with a small absolute tolerance:

    await expect(Math.abs(afterMove - afterWidth)).toBeLessThan(0.5);

These alternatives provide more flexibility and clarity in handling potential floating-point discrepancies.

})
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/basic-usage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
2 changes: 0 additions & 2 deletions examples/sites/demos/pc/app/split/collapsible-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ function collapsedChangeVertical(collapsed) {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid var(--ti-common-color-line-dividing);
}

.demo-split-pane {
Expand All @@ -51,7 +50,6 @@ function collapsedChangeVertical(collapsed) {

.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}
</style>
2 changes: 0 additions & 2 deletions examples/sites/demos/pc/app/split/collapsible.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid var(--ti-common-color-line-dividing);
}

.demo-split-pane {
Expand All @@ -59,7 +58,6 @@ export default {

.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}
</style>
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/disabled-drag-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.5)
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/disabled-drag.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/event-click-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ function rightBottomClick() {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/event-click.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/horizontal-collapse-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.5)
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/horizontal-collapse-left-top-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.5)
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/horizontal-collapse-left-top.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/horizontal-collapse.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/left-right-slot-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split2 = ref(0.4)
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/left-right-slot.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/moveend-event-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ function moveend() {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/moveend-event.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/movestart-event-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ function movestart() {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/movestart-event.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/moving-event-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ function moving() {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/moving-event.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/nested-use-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const split4 = ref(0.5)
<style scoped>
.split-nest {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
5 changes: 2 additions & 3 deletions examples/sites/demos/pc/app/split/nested-use.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test('嵌套使用', async ({ page }) => {
// 移动之后的宽度为:移动之前的宽度+50-分割线宽度的一半
const afterMove = leftWidth + 50 - centerWidth / 2
const { width: afterWidth } = await leftDiv.boundingBox()
await expect(afterMove).toBeCloseTo(afterWidth, 1)
await expect(Math.ceil(afterMove)).toEqual(Math.ceil(afterWidth))
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 the implications of using Math.ceil and toEqual.

The change from toBeCloseTo to Math.ceil and toEqual alters the assertion's behavior:

  1. It makes the test more strict by requiring exact equality after rounding up.
  2. It might catch issues previously masked by floating-point imprecisions.
  3. However, it could make the test more brittle if small variations are expected and acceptable.

Consider the following alternatives:

  1. Use Math.round instead of Math.ceil to handle cases where the value is closer to the lower integer.
  2. Implement a custom matcher that allows for a small tolerance while still using integer comparisons.

Example of a custom matcher:

expect(Math.round(afterMove)).toEqual(expect.closeTo(Math.round(afterWidth), 1));

This approach would maintain integer comparisons while allowing for a small tolerance.


// 测试左面板中的上下面板
const centerBtn1 = page.locator('.tiny-split-trigger-horizontal')
Expand All @@ -32,6 +32,5 @@ test('嵌套使用', async ({ page }) => {
await page.mouse.up()
const { height: afterHeight } = await topDiv.boundingBox()
const topMove = topHeight - 30 - verticalHeight / 2
await expect(topMove).toBeCloseTo(afterHeight, 1)
await page.waitForTimeout(300)
await expect(Math.ceil(topMove)).toEqual(Math.ceil(afterHeight))
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consistent change, consider generalizing the approach.

The change here is consistent with the previous modification, which is good for maintaining uniformity in the test assertions.

To improve maintainability and reduce duplication, consider:

  1. Creating a custom matcher or helper function for these comparisons. This would centralize the logic and make it easier to adjust in the future.

Example:

function expectRoundedEqual(actual: number, expected: number) {
  expect(Math.round(actual)).toEqual(Math.round(expected));
}

// Usage
expectRoundedEqual(topMove, afterHeight);
  1. If this pattern is used across multiple test files, consider adding it to a shared test utilities file.

This approach would make the tests more readable and easier to maintain, especially if you need to adjust the comparison logic in the future.

})
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/nested-use.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export default {
<style scoped>
.split-nest {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/split-mode-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ function moveend() {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/split/split-mode.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ test('分割方式', async ({ page }) => {
const { width: splitWidth } = await split.boundingBox()
const { width: centerWidth } = await centerBtn.boundingBox()
// 判断宽度是否和面板总宽度保持一致
await expect(splitWidth).toBeCloseTo(centerWidth, 1)
await expect(splitWidth).toBeCloseTo(centerWidth + 2)
})
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/split-mode.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/split-threshold-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.3)
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
4 changes: 2 additions & 2 deletions examples/sites/demos/pc/app/split/split-threshold.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ test('面板阈值', async ({ page }) => {
await page.mouse.move(x, y - 100)
await page.mouse.up()
const { height: afterHeight } = await topPanel.boundingBox()
await expect(50).toBeCloseTo(afterHeight, 1)
await expect(50).toEqual(Math.ceil(afterHeight))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider the implications of using exact equality for panel height.

The change from toBeCloseTo to toEqual with Math.ceil makes the test more strict. While this ensures precise behavior, it might lead to brittle tests if there are small variations in panel sizes due to browser rendering differences.

Consider using a small tolerance or range check instead. For example:

const expectedHeight = 50;
await expect(afterHeight).toBeGreaterThanOrEqual(expectedHeight - 1);
await expect(afterHeight).toBeLessThanOrEqual(expectedHeight + 1);

This approach maintains strictness while allowing for minor rendering variations.

await page.waitForTimeout(300)
// 向下滑动时判断下面面板高度是否为80
await triggerBtn.hover()
await page.mouse.down()
await page.mouse.move(x, y + 200)
const { height: bottomMove } = await bottomPanel.boundingBox()
await page.mouse.up()
await expect(80).toBeCloseTo(bottomMove, 1)
await expect(80).toEqual(Math.ceil(bottomMove))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consistent change in assertion, but consider variable naming.

The change to use toEqual with Math.ceil is consistent with the previous assertion. However, there's a naming issue to address.

  1. The variable bottomMove suggests movement, but it's storing the height. Consider renaming it to bottomHeight for clarity:
const { height: bottomHeight } = await bottomPanel.boundingBox()
await expect(80).toEqual(Math.ceil(bottomHeight))
  1. As mentioned in the previous comment, consider using a tolerance range instead of exact equality to make the test more robust against minor rendering variations.

})
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/split-threshold.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/three-areas-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const split2 = ref(0.25)
<style scoped>
.split-nest {
height: 300px;
border: 1px solid #d9d9d9;
}

.split-content {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/three-areas.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export default {
<style scoped>
.split-nest {
height: 300px;
border: 1px solid #d9d9d9;
}

.split-content {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/top-bottom-slot-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.3)
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/top-bottom-slot.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/trigger-simple-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const split1 = ref(0.5)
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/split/trigger-simple.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ test('简易模式', async ({ page }) => {
// 移动之后的宽度为:移动之前的宽度+50-分割线宽度的一半
const afterMove = leftPanelWidth + 100 - triggerBtnWidth / 2
const { width: afterWidth } = await leftPanel.boundingBox()
await expect(afterMove).toBeCloseTo(afterWidth, 1)
await expect(Math.ceil(afterMove)).toEqual(Math.ceil(afterWidth))
})
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/trigger-simple.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default {
<style scoped>
.split-v-model {
height: 200px;
border: 1px solid #d9d9d9;
}

.demo-split-pane {
Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/trigger-slot-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const TinyIconPause = IconPause()
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
1 change: 0 additions & 1 deletion examples/sites/demos/pc/app/split/trigger-slot.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export default {
<style scoped>
.demo-split {
height: 200px;
border: 1px solid #d9d9d9;
margin-bottom: 20px;
}

Expand Down
Loading
Loading