-
Notifications
You must be signed in to change notification settings - Fork 13
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
FIxes > Various Docs site issues #1574
Conversation
🦋 Changeset detectedLatest commit: a872661 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few outstanding issues:
Combobox:
- Async: inputValue is declared but value is never read, 'items' is of type 'unknown'.
- Custom Items Typescript: cannot find name uuid, Type '{ id: any; name: string; representation: string; }' is not assignable to type 'CustomComboboxItem'.
- Custom Items: cannot find name uuid, Type '{ id: number; name: string; representation: string; hex: string; }' is not assignable to type '{ id: any; name: any; representation: any; }'.
IconButton:
https://docs-preview-1574--upbeat-sinoussi-f675aa.netlify.app/api/icon-button/ is broken
gridJustifyContent="center" | ||
gridAlignItems="center" | ||
gridJustifyContent={GridJustifyContent.center} | ||
gridAlignItems={gridAlignItems.center} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gridAlignItems={gridAlignItems.center} | |
gridAlignItems={GridAlignItems.center} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Grid ✅
- Container ✅
- Stepper ✅
- Combobox ❌
-- Custom Items Typescript: Type '{ id: any; name: string; representation: string; }' is not assignable to type 'CustomComboboxItem'.
-- Custom Items: Type '{ id: number; name: string; representation: string; hex: string; }' is not assignable to type '{ id: any; name: any; representation: any; }'. - Hyperlink ✅
- IconButton ❌
-- Basic Usage: Property 'success' does not exist on type 'typeof ButtonColor'.
-- Event Handling type error - Toast ✅
- DatePicker ❌
-- Value: Type 'string' is not assignable to type 'Date'.
-- Minimum and Maximum Dates: Type 'string' is not assignable to type 'Date'.
-- Clearing the Date: Argument of type 'null' is not assignable to parameter of type 'Date'. - Dropdown ❌
-- OptionalDropdownMenuItem: Type '{ children: string; toggle: true; onClick: () => void; }' is not assignable to type 'IntrinsicAttributes & { toggle: any; dropdownMenuItemProps: any; }' - Form ✅
- FormGroup ❌
-- Inverse: Toggle is imported and not used - useFocusLock ✅
These 3 items were added to the ticket yesterday morning and have not been addressed yet:
13. ProgressBar ❌
14. RadioButton ❌
15. Select ❌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -25,7 +25,7 @@ import { LeadParagraph } from '../../components/LeadParagraph'; | |||
|
|||
An accordion is made up of four components: `Accordion`, `AccordionItem`, `AccordionButton` and `AccordionPanel`. The `Accordion` is the wrapper for all of the items. It can contain one or more `AccordionItems`, each of which should contain one `AccordionButton` and one `AccordionPanel`. An `AccordionButton` can be wrapped in an element with the role of heading (such as an h1-h6) to provide semantic structure to the document. | |||
|
|||
By default, multiple items will can be expanded. The `defaultIndex` prop, which takes an array of zero-based indices, can be used to set panels open by default. | |||
By default, multiple items are expandable. The `defaultIndex` prop which takes a of zero-based array can be used to set panels open by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is confusing
@@ -470,20 +470,25 @@ export function Example() { | |||
Using React's `forwardRef` feature you can gain access to the reference of the internal radio. | |||
|
|||
```tsx | |||
import React from 'react'; | |||
import React from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this example has been broken since 2.3.9, so it took me a second to understand what it's trying to achieve. I think it's trying to showcase how you can programmatically, with a ref (radioRef
), set or unset the radio button option one. I think we can achieve this and clean this example a little bit better with something like this:
import React from 'react';
import {
RadioGroup,
Radio,
Button,
ButtonColor,
ButtonGroup,
} from 'react-magma-dom';
export function Example() {
const radioRef = React.useRef();
const [radioGroupVal, setRadioGroupVal] = React.useState(null);
function handleSelectOne() {
if (!radioRef.current || radioRef.current.checked) {
setRadioGroupVal(null);
} else if (!radioRef.current.checked) {
setRadioGroupVal("1");
}
}
function handleFocusOne() {
radioRef.current.focus();
}
return (
<>
<RadioGroup
labelText="Forward ref usage"
id="forwardRefGroup"
name="forwardRef"
value={radioGroupVal}
>
<Radio
ref={radioRef}
id="forwardRefRadio1"
labelText="Option one label"
value="1"
/>
<Radio id="forwardRefRadio2" labelText="Option two label" value="2" />
</RadioGroup>
<ButtonGroup>
<Button onClick={handleSelectOne}>Toggle option 1 selection</Button>
<Button color={ButtonColor.secondary} onClick={handleFocusOne}>
Focus option 1
</Button>
</ButtonGroup>
</>
);
}
What I tested
|
The IconButton error is only visible because our default codesandbox doesn't use the latest pre-release version. I have confirmed that if you change the package to the latest, the error disappears. |
Issue: # 1554
What I did
Cleaned up CodeSandbox examples for: Grid, Container, Stepper, Combobox, Hyperlink, IconButton, Toast, DatePicker, Dropdown, Form, FormGroup, & useFocusLock
Checklist
How to test
Go to: https://next--upbeat-sinoussi-f675aa.netlify.app/api/
1. Grid
Basic Usage example
2. Container
All examples
3. Stepper
Step label example
Completed Step Description example
4. Combobox
Label Position example
Async example
Custom Items example
5. Hyperlink
Icon example
6. IconButton
Basic Usage example
Inverse example
7. Toast
Toast Documentation
There are a couple items in the "Accessibility Considerations" section that need to be fixed.
The toast is displayed in the bottom-left corner of the window, not the center
"Where ever" should be "Wherever".
8. DatePicker
Default Date example
Value example
The description under Keyboard Navigation is no longer accurate now that we've removed ?
If you would like to see all of the options for keyboard navigation click on the ? button in the bottom right corner of the DatePicker or press the ? key. should be removed
9. Dropdown
DropdownSplitButton example
Custom DropdownMenuItem Wrappers example
Expandable Menu examples have a new bug so the CodeSandbox will still show the error with icon:
#1573
10. Form
Form Props should be heading 2 ## instead of 1 so it can show up on the side nav
11. FormGroup
Visually Hidden Label, Custom Styles and Labeling Form Groups all use name prop which is not needed
12. useFocusLock
Optional Header Without Focusable Element example (removed references of Input, Button, Heading)