[Storybook] Update Storybook to move DialogV1 to deprecated #4758
Conversation
|
size-limit report 📦
|
There was a problem hiding this comment.
Also curious if we have alignment to start the engineering portion of the deprecation cycle for DialogV1.
As far as I know we paused on the work for Dialog v2. Not sure exactly when it is going to be prioritised. That means, it might get a little tricky to deprecate this Dialog when we don't have a "Ready" Dialog component (Alpha or Beta).
I don't know if we want to go down the road to try something different for this. I'll tag @siddharthkp to see what he thinks.
Thanks! It's a bit awkward because it's not deprecated deprecated. But, for the sake of clarity, we do need to recommend one Dialog over the other. Dialog v2 is still in My gut reaction is to promote the new component, but I checked and neither of the Dialogs are reviewed for accessibility 😓 I would like (someone) to look into this a bit more from the view of design and accessibility and bless one of them, and then we can start the process of deprecating the other one 😇 @tbenning Maybe you already have? |
So far whenever anyone has asked in Slack support channels, we have been recommending folks use Dialog over DialogV1. Though neither have been reviewed for accessibility, I'm curious if the newer one is the path forward and that we will review this one? Maybe folks in accessibility will have more context. We can elevate this discussion with more folks in Primer, but this feels like a core decision we need to make since Dialog is an important and high usage component. |
Is getting this component reviewed for accessibility the only thing that's blocking us from promoting Dialog v2? I tried looking for a tracking issue for Dialog v2 but couldn't find it. I think if this is the only thing that's blocking us, and there isn't any reason why we need to wait on getting it reviewed, we could potentially go about that process now 🤔 |
That and the fact we haven't scheduled time for it yet. Accessibility remediations might be the push it needs
Yes! That's why I wanted to pick the version that has more usage, but it feels pretty spread out: Dialog v1: 206 instances So we can pick either v1 or v2 to remediate. (Assumption that needs validating: we would be able to remediate one of these in-place without breaking changes, instead of creating a new component Dialog v3 😭) Another deciding factor can be design. Is the design of v2 more in line with our direction? |
@siddharthkp - v2 is more in line with the design direction |
|
Since we're pushing people towards Dialog v2, I'm comfortable moving this to the "Deprecated" section of Storybook even though it's technically still in the main package. I'd love to see us get Dialog v2 into the main package like @siddharthkp described
|
| @@ -7,7 +7,7 @@ import {default as Dialog} from './Dialog' | |||
| /* Dialog Version 1*/ | |||
|
|
|||
| export default { | |||
| title: 'Components/DialogV1', | |||
| title: 'Deprecated/Components/DialogV1', | |||
There was a problem hiding this comment.
If we're comfortable doing this for Dialog, I think it would be good to do the same for Tooltip. This was mentioned during today's Primer Patterns working session.
There was a problem hiding this comment.
Yup! I had drafted a PR for this and just flipped it to review
|
Quick update: I ran this past folks in Primer Patterns today and have support from the group for moving it to deprecated. This change feels low risk since the bulk of the risk will be once we start properly deprecating things, this just pre-empts that work. |
Thanks for sharing that @tbenning! I understand we are looking into gradually doing that deprecation work but It is confusing to me to have a component in the Deprecated folder in Storybook but nothing else indicates its deprecation in the code or in the tools. We have discussed about deprecation process before and there are a couple of other things that we think are useful in a deprecation process. For example the below items are suitable for this case.
Is there anything blocking us to do such things for Dialog? Especially the |
+1 to that Just wanted to add that it would be confusing to think you are importing the Dialog you see in storybook with This is a bit messy, so thanks @tbenning for starting work on this, otherwise it wouldn't get done 😓 |
|
I'm going to close this PR in the meantime, since we're now looking at component status and how we communicate different stages of deprecation. I will likely come back with a PR that has a banner warning about deprecation instead. |
Closes #
This is part of an ongoing deprecation path for DialogV1. I can't link the issue since my work computer is still on the fritz, but it's a part of the Streamline Primer initiative.
Problem/Solution
This a quick patch to help the Developer Experience and provide clarity since we keep getting questions in #primer slack support channels about which Dialog to use since they see 2 in Storybook. This should provide some immediate relief and clarity for the team as we continue to remediate V1 and V2. Ideally, we'd follow the deprecation guide here and move it to the deprecated folder and update all of the tests involved.
Note: I tried to start moving DialogV1 into the deprecation folder, etc. but started breaking a number of tests and need a bit of help to go down this path. Also curious if we have alignment to start the engineering portion of the deprecation cycle for DialogV1.
Changelog
Changed
Moved
Components/DialogV1->Deprecated/Components/DialogV1Rollout strategy
This is a brief docs update before we follow the full deprecation path.
Testing & Reviewing
Merge checklist