fix(NotificationsPanel): new date-time formatting function#6904
Conversation
paul-balchin-ibm
requested review from
elycheea and
makafsal
and removed request for
a team
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
paul-balchin-ibm
added
component: NotificationsPanel
status: needs release review 👀
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6904 +/- ##
=======================================
Coverage 81.73% 81.74%
=======================================
Files 401 401
Lines 13025 13030 +5
Branches 4293 4296 +3
=======================================
+ Hits 10646 10651 +5
Misses 2379 2379
🚀 New features to boost your workflow:
|
paul-balchin-ibm
added
status: one more review 👀
and removed
status: needs release review 👀
| "vite": "^6.0.7" | ||
| }, | ||
| "dependencies": { | ||
| "@carbon/utilities": "^0.4.0", |
There was a problem hiding this comment.
Since this is being used by the actual NotificationsPanel component, we would need to move this into the packages/ibm-products/package.json.
However, re-reading the original issue, it looks like we actually intended for the user to use dateTimeFormat, which means we would probably want to use the utility in the data or story rather than in the component itself.
There was a problem hiding this comment.
Hi @elycheea, based on recent meetings, is this still the case?
There was a problem hiding this comment.
Yep. Move it to our ibm-products/package.json! (Only for the first part those. Second part you can disregard.)
|
Draft mode - follow up discussions After further discussion, there are still open questions on how to deprecate the old props gracefully. We can keep the old props, mark them as deprecated, and remove the But, if the app developer was providing translations using the old props, these will now only render the default English, since the old functionality has been removed. For the app developer to specify other languages, they will need to know to use the new Ready for review - Post discussion fix Removed default value for |
| /** | ||
| * The `NotificationsPanel` sets expectations on the behavior for notifications, | ||
| * allowing the user to view and interact with them all in one place. | ||
| * | ||
| * #### Updated localization feature for the "time ago" timestamp | ||
| * | ||
| * The new localization reduces a dev's effort to apply language | ||
| * support for the "time ago" timestamp applied to each notification | ||
| * by using the new Carbon utility [dateTimeFormat()](https://github.com/carbon-design-system/carbon/blob/main/packages/utilities/src/dateTimeFormat/README.md) function. | ||
| * It will provide relative timestamps that adhere to Carbon's | ||
| * [date and time guidance](https://pages.github.ibm.com/carbon/ibm-products/guidelines/content/date-and-time/). | ||
| * | ||
| * **Step 1:** ignore the following _deprecated_ props: `daysAgoText`, | ||
| * `hourAgoText`, `hoursAgoText`, `minuteAgoText`, `minutesAgoText`, | ||
| * `monthAgoText`, `monthsAgoText`, `nowText`, `secondsAgoText`, | ||
| * `yearAgoText`, `yearsAgoText`, `yesterdayAtText`. | ||
| * | ||
| * **Step 2:** provide the `dateTimeLocale` prop with a valid locale—such as "de" or "fr-CA"—and it will do the work. | ||
| * You must provide a value, otherwise the deprecated props above will be used instead. | ||
| */ |
There was a problem hiding this comment.
I think we’d typically just include this in the actual documentation (StoryDocs or mdx) rather than in the component file.
I would consider reversing the order for adopting the new props though. Rather than “ignore” as a task, I think it makes more sense to 1) adopt, 2) remove.
To adopt the new localization:
Step 1: Provide a valid locale to the
dateTimeLocaleprop, such asdeorfr-CA.
Step 2: Remove the now deprecated props:daysAgoText,hourAgoText,hoursAgoText,minuteAgoText,minutesAgoText,monthAgoText,monthsAgoText,nowText,secondsAgoText,yearAgoText,yearsAgoText,yesterdayAtText.If you do not provide a locale, the deprecated props will be applied instead.
There was a problem hiding this comment.
<StoryDocsPage /> is picking up this comment and dropping into the Overview section on the Docs page. I can't find an example of telling <StoryDocsPage /> to fill in the Overview section directly.
There was a problem hiding this comment.
RE this comment, <StoryDocsPage /> is definitely designed to pick up the component comment as the "Overview":
Here's where it's output
However, altDescription is ignored if used as a prop.
Still looking.
There was a problem hiding this comment.
I actually imagine we’ll eventually move back to MDX through the docs epic 👀, but for now, I’d just focus on revising the docs you added rather than unpacking the StoryDocsPage. I want to say that was a workaround when we ran into issues with MDX when we first moved to Storybook 7 or 8? 🤔
There was a problem hiding this comment.
Works for me 👍 I'll stop digging into StoryDocsPage if we're planning to move back to mdx.
elycheea
left a comment
elycheea
left a comment
There was a problem hiding this comment.
Changes look good but since I see there’s a conflict, I’ll add one tiny fix to the updated docs. (Just colon instead of period.)
…cationsPanel.tsx Co-authored-by: elysia <elysia.hwang@gmail.com>
|
I noticed the doc change while resolving the conflict. Both are completed. |
3 participants
Closes #6761
Direct link to NotificationsPanel Story
Introduced new Carbon utilities dateTimeFormat() function, which follows official date and time and language guidance.
dateTimeFormat()and 2 new props replaces:* These older props are now marked as deprecated and remain functional.
What did you change?
Added
@carbon/utilities.How did you test and verify your work?