Skip to content

Format titleCell with locale#312

Open
hensansi wants to merge 8 commits intoreact-component:masterfrom
hensansi:format-titleCell-with-locale
Open

Format titleCell with locale#312
hensansi wants to merge 8 commits intoreact-component:masterfrom
hensansi:format-titleCell-with-locale

Conversation

@hensansi
Copy link
Copy Markdown

I encountered the same issue as ant-design/ant-design#31339

Alternative solution to #273

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 26, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/picker/DANeVMS6fUmJdFfUwGtQUK8tKtQN
✅ Preview: https://picker-git-fork-hensansi-format-titlecel-79de3d-react-component.vercel.app

@hensansi
Copy link
Copy Markdown
Author

Regarding the coverage, it is a breaking change due to the current locales,
https://github.com/react-component/picker/blob/master/src/locale/en_US.ts#L19

I will look into why rc-picker-cell-end disappears

Comment thread tests/util.spec.tsx
import { toArray } from '../src/utils/miscUtil';
import { isSameTime, isSameDecade } from '../src/utils/dateUtil';
import { getMoment } from './util/commonUtil';
import koKR from '../src/locale/ko_KR';
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I only used koKR to keep some continuation with the previous implementation 'YYYY-MM-DD'

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 28, 2021

Codecov Report

Merging #312 (a71f9f9) into master (13e10ec) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head a71f9f9 differs from pull request most recent head b7dc951. Consider uploading reports for the commit b7dc951 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
- Coverage   99.52%   99.52%   -0.01%     
==========================================
  Files          45       45              
  Lines        2118     2117       -1     
  Branches      627      627              
==========================================
- Hits         2108     2107       -1     
  Misses          8        8              
  Partials        2        2              
Impacted Files Coverage Δ
src/panels/DatePanel/DateBody.tsx 96.00% <100.00%> (ø)
src/panels/PanelBody.tsx 100.00% <100.00%> (ø)
src/utils/timeUtil.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13e10ec...b7dc951. Read the comment docs.

The locale should be present if picker isn't a year picker
@hensansi hensansi marked this pull request as ready for review October 29, 2021 09:20
Comment thread tests/__snapshots__/panel.spec.tsx.snap Outdated
<tr>
<td
class="rc-picker-cell"
title="1990-08-26"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think that this behavior wise is a breaking change, so it needs to be handled/notified carefully although at the same time it will give a better experience to the user.

@hensansi
Copy link
Copy Markdown
Author

hensansi commented Oct 29, 2021

I just noticed that there are harcoded defaults in other places/not using the locale (enGB) e.g.

DeepinScreenshot_select-area_20211029162421

RangePicker

export function getDefaultFormat<DateType>(

I can try to understand how we can use the locales for that too, I assumed that the code was using a default locale but that's not the case. Can you give me some hint on why it was structured this way/can I bring some suggestion to the table regarding this?

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.

1 participant