chore(GlDisclosureDropdown): Simplify arg creation
What does this MR do?
chore(GlDisclosureDropdown): Simplify arg creation
Storybook is already able to infer prop args via the component
property of the default export of a story file. There's no need to
explicitly read them ourselves.
This led to a problem for a lazily defined default value in !3244 (comment 1259093614).
It may be this pattern of explicitly reading default values arose before Storybook inferred component props.
Before | After |
---|---|
![]() |
![]() |
Discussion
Pros
- Less code
🎉 - Avoids a class of bugs (incorrect default value fetching).
- It's clearer which props are not set by a given story, since they have a
Set boolean/string/number/etc
button instead of the default value in the control panel. (See for instancedisabled
in the screenshots above.)
Cons
-
Prop order is different, probably for each story, depending on the explicit args set for each story. The order is now determined by:
- Explicit arg source order.
- Remaining inferred args from component props in source order.
This is technically the same as before. The difference now is that there are fewer explicit args set, so the source order of component props mattered less.
-
You can no longer at a glance see what the default value is for an optional prop in the controls panel. For instance, looking at the
disabled
control in the screenshots above, before you can see it'sfalse
by default, whereas now it's just aSet boolean
button, which reveals a toggle. Though, the toggle does then show the correct default (false
in this case).
Alternatives
- We could create a story helper that implements the prop detection and default value fetching, avoiding things like !3277 (merged).
Does this MR meet the acceptance criteria?
Conformity
-
Code review guidelines. -
GitLab UI's contributing guidlines. -
If it changes a Pajamas-compliant component's look & feel, the MR has been reviewed by a UX designer. -
If it changes GitLab UI's documentation guidelines, the MR has been reviewed by a Technical Writer. -
If the MR changes a component's API, integration MR(s) have been opened in the following projects to ensure that the @gitlab/ui
package can be upgraded quickly after the changes are released:-
GitLab: mr_url -
CustomersDot: mr_url -
Status Page: mr_url
-
-
Added the ~"component:*"
label(s) if applicable.
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
Security reports checked/validated by a reviewer from the AppSec team
Accessibility
If this MR adds or modifies a component, take a few moments to review the following:
-
All actions and functionality can be done with a keyboard. -
Links, buttons, and controls have a visible focus state. -
All content is presented in text or with a text equivalent. For example, alt text for SVG, or aria-label
for icons that have meaning or perform actions. -
Changes in a component’s state are announced by a screen reader. For example, changing aria-expanded="false"
toaria-expanded="true"
when an accordion is expanded. -
Color combinations have sufficient contrast.