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

fix(date-picker): add dayjs updateLocale plugin to support correct week numbers #1706

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Summer-Shen
Copy link

@Summer-Shen Summer-Shen commented Jan 11, 2024

🤔 这个 PR 的性质是?

  • 日常 bug 修复
  • 新特性提交
  • 文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • CI/CD 改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他

🔗 相关 Issue

Tencent/tdesign-vue-next#3785

💡 需求背景和解决方案

更新 dayjs 已有的语言配置 以获取正确的周数。

  • firstDayOfWeek 为一周第 1 天,则这周的第 4 天我们记为星期 x
  • 本年度第一个星期 x 所在的星期是本年度的第一周。

date-picker-week-number

参考:iamkun/dayjs#2237

📝 更新日志

  • fix(date-picker): 添加 dayjs updateLocale 插件,以支持正确的周数

  • 本条 PR 不需要纳入 Changelog

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

@chaishi
Copy link
Collaborator

chaishi commented Jan 13, 2024

非常感谢 PR 🌹 稍后会有同事帮忙验证

@azx1573
Copy link
Contributor

azx1573 commented Jan 13, 2024

非常感谢 PR 🌹 稍后会有同事帮忙验证

非常感谢你的pr 🌹
原来的问题是:当鼠标悬浮或者点击时,输入框中的值是准确符合实际的,只是采用的ISO的标准导致对周的定义跟面板中的展示形式不一致,比如不管firstDayOfWeek传入什么,在zh-ch下,始终将周一作为本周第一天,因此不管firstDayOfWeek传入啥,24年的1号到7号都是被认为是24年的第一周,但是周数的逻辑是正确无误的,比如23年12月31号就是23年第52周。
image
采用了你这个方法后,发现当日期选择了12月31号时,周数的逻辑有点问题,23年12月31号成了23年的第一周,辛苦再看一下?
image
@Summer-Shen

@Summer-Shen
Copy link
Author

Summer-Shen commented Jan 13, 2024

@azx1573 感谢!想问下从选择周的角度来说,是否点在 2023-12-31 和 2024-01-01 应该显示两种不同的结果,还是统一为 2024 年第 1 周?

@azx1573
Copy link
Contributor

azx1573 commented Jan 13, 2024

@azx1573 感谢!想问下从选择周的角度来说,是否点在 2023-12-31 和 2024-01-01 应该显示两种不同的结果,还是统一为 2024 年第 1 周?

  1. 关于以什么为主:确切的来说,站在用户的角度(这里应该为该组件的使用者)来说,比如我选择23年12月31日,onChange返回给ta的结果应该是23年的52周,具体来说应该是以实际情况为主,另外这里还有个bug就是面板的高亮逻辑是用实际值去判断的,所以再次展开时会出现面板高亮和实际不符的情况。所以在设置了firstDayOfWeek的情况下,只需以实际情况为主,不然会导致结果不是组件使用者想要的。
  2. 理想的预期:以firstDayOfWeek为6为例
    1> 如果我选择了23年12月30或者31,值为23年52周,高亮当前日期所在行
    2> 如果我选择了24年1月的1号-24年1月的5号,值为24年第一周,依然高亮这行,因为这7天为同一周。
    @Summer-Shen

@Summer-Shen
Copy link
Author

Summer-Shen commented Jan 13, 2024

@azx1573 感谢,那面板上周数如何标识?比如说当前面板上是 2023 年 12 月,那最后一周左侧应该是 52 周还是 1 周?

这里还有个bug就是面板的高亮逻辑是用实际值去判断的,所以再次展开时会出现面板高亮和实际不符的情况。

我测试了一下 tdesign-vue-next 好像没有上述这个问题。

@azx1573
Copy link
Contributor

azx1573 commented Jan 13, 2024

@azx1573 感谢,那面板上周数如何标识?比如说当前面板上是 2023 年 12 月,应该是 52 周还是 1 周?

这里还有个bug就是面板的高亮逻辑是用实际值去判断的,所以再次展开时会出现面板高亮和实际不符的情况。

我测试了一下 tdesign-vue-next 好像没有上述这个问题。

这是我刚想要说的一点,
突然发现之前这里是52,现在变成1开始了。
你是直接改的getweeks,而面板数组就是通过这个方法生成的,所以日期面板数组的值及顺序都被改了哦,首先这可能会对已使用这个组件的业务功能会造成影响哈(具体以实际评估结果为准,只是推测可能)。

之前存在高亮这个问题的你可以试试,但是你将这个数组顺序改了后就不存在了,因为23年31号被认为是23年第一周😂,等号前面值为1,1-5号是24年第一周,值也为1,选这周的任何一个值value都为1,后面是数组第一列表示周的值,之前52现在变成1,两个值相等所以高亮的问题不存在了😂
image
image
image
@Summer-Shen

@Summer-Shen
Copy link
Author

你是直接改的getweeks,而面板数组就是通过这个方法生成的,所以日期面板数组的值及顺序都被改了哦,首先这可能会对已使用这个组件的业务功能会造成影响哈(具体以实际评估结果为准,只是推测可能)。

@azx1573 只针对这一点来说,请问如果抽出一个函数计算周的序号可以么?还是从其他地方(比如各个技术栈的展示)入手比较合适?

@azx1573
Copy link
Contributor

azx1573 commented Jan 13, 2024 via email

@Summer-Shen
Copy link
Author

Summer-Shen commented Jan 13, 2024

简单测试了一下,dayjs 的 format 输出如下:

const dayjs = require("dayjs");
const weekOfYear = require("dayjs/plugin/weekOfYear");
const updateLocale = require("dayjs/plugin/updateLocale");
const advancedFormat = require("dayjs/plugin/advancedFormat");

dayjs.extend(weekOfYear);
dayjs.extend(updateLocale);
dayjs.extend(advancedFormat);

dayjs.updateLocale("zh-cn", {
  weekStart: 7, // week starts at Sunday
});

function parseDateToWeekNumber(date) {
  return dayjs(date).locale("zh-cn").format("YYYY-wo");
}

console.log(parseDateToWeekNumber("2025-12-31")); // 2025-1st
console.log(parseDateToWeekNumber("2026-01-01")); // 2026-1st

可能打算去给 dayjs 提 issue;and/or 我们要自己实现 format.ts 中的逻辑了。

另一方面来说,ISO 8601 的定义是基于一周的第一天是周一,我们可能需要讨论一下这个定义、我再考虑一下。

@azx1573
Copy link
Contributor

azx1573 commented Jan 13, 2024

简单测试了一下,dayjs 的 format 输出如下:

const dayjs = require("dayjs");
const weekOfYear = require("dayjs/plugin/weekOfYear");
const updateLocale = require("dayjs/plugin/updateLocale");
const advancedFormat = require("dayjs/plugin/advancedFormat");

dayjs.extend(weekOfYear);
dayjs.extend(updateLocale);
dayjs.extend(advancedFormat);

dayjs.updateLocale("zh-cn", {
  weekStart: 7, // week starts at Sunday
});

function parseDateToWeekNumber(date) {
  return dayjs(date).locale("zh-cn").format("YYYY-wo");
}

console.log(parseDateToWeekNumber("2025-12-31")); // 2025-1st
console.log(parseDateToWeekNumber("2026-01-01")); // 2026-1st

可能打算去给 dayjs 提 issue;and/or 我们要自己实现 format.ts 中的逻辑了。

另一方面来说,ISO 8601 的定义是基于一周的第一天是周一,我们可能需要讨论一下这个定义、我再考虑一下。

ISO 8601 的标准我们也没法改变,antd我大概看了下印象中好像都不支持自定义周起始日,tdesigin已经支持了并且内外部已经有不少项目使用了这个事实也是没法改变的😂。你下午提了后,我也用了两个demo,一个是你这边的代码,另一个是独立使用dayjs的,粗略验证了下dayjs里部分特殊日期的值确是有问题的,所以想着看看你这边能不能有更独特,更优更新颖的解法。

@Summer-Shen
Copy link
Author

Summer-Shen commented Feb 13, 2024

@azx1573

最近做了一些测试和思考,考虑到既有的值格式为 YYYY-wo,可能的方案是不修改 value 的格式,仅修改:

  • 如何比较两个日期是否在同一周。在跨年前后,这个比较有两种含义,即

    • 两个日期是否对应一个 YYYY-wo,以及

    • 两个日期是否在 firstDayOfWeek 的条件下处于同一周,比如各个技术栈仓库中

      parseToDayjs(value, format).locale(dayjsLocale).week() === 
      parseToDayjs(targetValue, format).locale(dayjsLocale).week()
  • 如何计算周数,比如 format.ts 文件的 parseToDayjs 函数中。

不过这也仍然是 breaking change:当 firstDayOfWeek 不是默认值时,原有 hover 时输入框的逻辑对周一及之后产生准确的周数,但对周日及以前则产生错误结果、也就是上一周。因此对于用户来说一个 YYYY-wo 值所指向的时间范围是发生了变化的,无论如何修改都难以避免。

另一方面而言,我注意到 dayjs 其实提供了另一种 format gggg可以代替 YYYY 表示日期对应周数的年份

从这两点来说,更好的方案可能是:

  • 给定一个日期,找出在这个日期之前最近的 firstDayOfWeek,记这一天为 startOfWeek
  • 然后取 startOfWeek.week() 作为周数,取 startOfWeek.format("gggg-wo") 作为输出值。

不知道您怎么看?感谢!

另外之后可能会重开一个 PR,之前的 solution 爆炸半径大了些。

@azx1573
Copy link
Contributor

azx1573 commented Feb 17, 2024

@azx1573

最近做了一些测试和思考,考虑到既有的值格式为 YYYY-wo,可能的方案是不修改 value 的格式,仅修改:

  • 如何比较两个日期是否在同一周。在跨年前后,这个比较有两种含义,即

    • 两个日期是否对应一个 YYYY-wo,以及
    • 两个日期是否在 firstDayOfWeek 的条件下处于同一周,比如各个技术栈仓库中
      parseToDayjs(value, format).locale(dayjsLocale).week() === 
      parseToDayjs(targetValue, format).locale(dayjsLocale).week()
  • 如何计算周数,比如 format.ts 文件的 parseToDayjs 函数中。

不过这也仍然是 breaking change:当 firstDayOfWeek 不是默认值时,原有 hover 时输入框的逻辑对周一及之后产生准确的周数,但对周日及以前则产生错误结果、也就是上一周。因此对于用户来说一个 YYYY-wo 值所指向的时间范围是发生了变化的,无论如何修改都难以避免。

另一方面而言,我注意到 dayjs 其实提供了另一种 format gggg可以代替 YYYY 表示日期对应周数的年份

从这两点来说,更好的方案可能是:

  • 给定一个日期,找出在这个日期之前最近的 firstDayOfWeek,记这一天为 startOfWeek
  • 然后取 startOfWeek.week() 作为周数,取 startOfWeek.format("gggg-wo") 作为输出值。

不知道您怎么看?感谢!

另外之后可能会重开一个 PR,之前的 solution 爆炸半径大了些。

非常感谢,年后一起看看❤️🌹 @Summer-Shen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants