Skip to content

Hotfixes: Update SelectMenu.Item type & extend Box in Position #803

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.

Sign up for GitHub

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

Merged
emplums merged 5 commits into from
May 8, 2020
Merged

Hotfixes: Update SelectMenu.Item type & extend Box in Position #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
17f08f5
make as prop optional
May 7, 2020
06a6607
give Position components flex props by extending Box
May 7, 2020
206e450
update types
May 7, 2020
9d9c12c
typo
May 7, 2020
7b34d3c
update docs
May 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/content/Position.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The Position component is a wrapper component that gives the containing componen

## System props

Position components get `POSITION`, `LAYOUT` and `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.
Position components get `POSITION`, `LAYOUT`, `FLEX, and `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.

## Component props

Expand Down
6 changes: 3 additions & 3 deletions docs/content/SelectMenu.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ Used to wrap the content in a `SelectMenu`.

### Right-aligned modal

Use the `align='right'` prop to align the modal to the right. Note that this only modifies alignment for the modal, and not the SelectMenu itself.
Use the `align='right'` prop to align the modal to the right. Note that this only modifies alignment for the modal, and not the SelectMenu itself. You will need to wrap the SelectMenu in a relatively positioned element for this to work properly.

```jsx live
<Flex justifyContent="flex-end" as={Relative}>
<Relative display="flex" justifyContent="flex-end">
<SelectMenu>
<Button as="summary">Projects</Button>
<SelectMenu.Modal align="right">
Expand All @@ -104,7 +104,7 @@ Use the `align='right'` prop to align the modal to the right. Note that this onl
</SelectMenu.List>
</SelectMenu.Modal>
</SelectMenu>
</Flex>
</Relative>
```

### System Props
Expand Down
18 changes: 4 additions & 14 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,7 @@ declare module '@primer/components' {
extends StyledSystem.BordersProps,
StyledSystem.BoxShadowProps {}

interface PositionProps
extends BaseProps,
StyledSystem.PositionProps,
StyledSystem.ZIndexProps,
StyledSystem.TopProps,
StyledSystem.RightProps,
StyledSystem.BottomProps,
StyledSystem.LeftProps {}
Comment on lines -32 to -37
Copy link
Copy Markdown
Author

@emplums emplums May 7, 2020

Choose a reason for hiding this comment

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

I removed all of these because they are already included in the definition for StyledSystem.PositionProps

Copy link
Copy Markdown
Author

@emplums emplums May 7, 2020

Choose a reason for hiding this comment

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

I removed BaseProps because the two components that extend PositionProps (Position & Popover) already are including BaseProps in their definition

interface PositionProps extends StyledSystem.PositionProps {}

export interface BoxProps
extends BaseProps,
Expand Down Expand Up @@ -273,10 +266,7 @@ declare module '@primer/components' {
}

export interface PositionComponentProps
extends PositionProps,
CommonProps,
LayoutProps,
Omit<React.HTMLAttributes<HTMLDivElement>, 'color'> {}
Comment on lines -277 to -279
Copy link
Copy Markdown
Author

@emplums emplums May 7, 2020

Choose a reason for hiding this comment

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

these are all included in BoxProps

extends PositionProps, BoxProps {}

export const Relative: React.FunctionComponent<PositionComponentProps>
export const Absolute: React.FunctionComponent<PositionComponentProps>
Expand All @@ -297,10 +287,10 @@ declare module '@primer/components' {
selected?: boolean;
}
interface SelectMenuItemAsButtonProps extends SelectMenuItemCommonProps, Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, 'color'> {
as: "button"
as?: "button"
}
interface SelectMenuItemAsAnchorProps extends SelectMenuItemCommonProps, Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, 'color'> {
as: "a"
as?: "a"
}
export type SelectMenuItemProps = SelectMenuItemAsButtonProps | SelectMenuItemAsAnchorProps;

Expand Down
16 changes: 7 additions & 9 deletions src/Position.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import React from 'react'
import PropTypes from 'prop-types'
import styled from 'styled-components'
import {COMMON, LAYOUT, POSITION} from './constants'
import {POSITION} from './constants'
import Box from './Box'
import theme from './theme'
import sx from './sx'

const Position = styled.div`
${LAYOUT}
${COMMON}
${POSITION}
const Position = styled(Box)`
${POSITION};
${sx};
`

Expand All @@ -17,11 +16,10 @@ Position.defaultProps = {
}

Position.propTypes = {
...COMMON.propTypes,
...LAYOUT.propTypes,
...Box.propTypes,
...POSITION.propTypes,
...sx.propTypes,
theme: PropTypes.object
theme: PropTypes.object,
...sx.propTypes
}

function withPosition(position) {
Expand Down