Skip to content
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

Migrate SimpleTable.tsx to chakra #8744

Merged
merged 8 commits into from
Jan 5, 2023
Merged

Migrate SimpleTable.tsx to chakra #8744

Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
709a619
refactor: SimpleTable to chakra
amit-ksh Nov 27, 2022
fca9999
making table row more accessible
amit-ksh Dec 9, 2022
61d9fb0
used chakra variable for border radius
amit-ksh Dec 23, 2022
3560ef0
Update src/components/SimpleTable.tsx
amit-ksh Dec 23, 2022
7fd9a0d
Merge branch 'ethereum:dev' into chakra/simpletable
amit-ksh Dec 23, 2022
180694b
center all tds
amit-ksh Dec 23, 2022
b8a6c03
always rendering `tr` element
amit-ksh Dec 24, 2022
c0bae05
remove ineffective styles
amit-ksh Dec 27, 2022
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
270 changes: 119 additions & 151 deletions src/components/SimpleTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,114 +1,20 @@
import React from "react"
import styled from "@emotion/styled"

import Link from "./Link"
import Translation from "./Translation"
import { FakeLinkExternal } from "./SharedStyledComponents"
import { TranslationKey } from "../utils/translations"

const Table = styled.div`
display: grid;
grid-template-columns: repeat(3, auto);
grid-template-rows: auto;
margin: 2rem 0rem;
border-radius: 2px;
border: 1px solid ${(props) => props.theme.colors.text};
background-color: ${(props) => props.theme.colors.background};
margin-bottom: 2rem;
min-width: 720px;
`

const StyledFakeLinkExternal = styled(FakeLinkExternal)`
color: ${(props) => props.theme.colors.text200};
&:after {
margin-left: 0.125em;
margin-right: 0.3em;
display: inline;
content: "↗";
transition: all 0s ease-in-out;
font-style: normal;
}
&:hover {
&:after {
transform: translate(0em, 0em);
}
}
`

const Cell = styled.div`
width: 100%;
display: flex;
align-items: center;
margin-right: 1rem;
`

const HeaderCell = styled.div`
width: 100%;
display: flex;
align-items: center;
text-transform: uppercase;
color: ${(props) => props.theme.colors.text200};
margin-right: 1rem;
`

const Header = styled(StyledFakeLinkExternal)`
grid-column: 1 / -1;
display: grid;
grid-template-columns: 1fr 1fr 1fr;
text-decoration: none;
display: flex;
box-shadow: 0 1px 1px ${(props) => props.theme.colors.text};
background: ${(props) => props.theme.colors.ednBackground};
margin-bottom: 1px;
padding: 1rem;
width: 100%;
`

const LinkRow = styled(Link)`
grid-column: 1 / -1;
display: grid;
grid-template-columns: 1fr 1fr 1fr;
text-decoration: none;
display: flex;
color: ${(props) => props.theme.colors.text};
box-shadow: 0 1px 1px ${(props) => props.theme.colors.text};
padding: 1rem;
width: 100%;
&:hover {
text-decoration: none;
box-shadow: 0 0 1px ${(props) => props.theme.colors.text};
background: ${(props) => props.theme.colors.primary200};
color: ${(props) => props.theme.colors.black300};
}
&:after {
color: ${(props) => props.theme.colors.primary};
}
`

const Row = styled.div`
grid-column: 1 / -1;
display: grid;
grid-template-columns: 1fr 1fr 1fr;
text-decoration: none;
display: flex;
color: ${(props) => props.theme.colors.text};
box-shadow: 0 1px 1px ${(props) => props.theme.colors.text};
padding: 1rem;
width: 100%;
&:hover {
box-shadow: 0 0 1px ${(props) => props.theme.colors.text};
background: ${(props) => props.theme.colors.primary200};
color: ${(props) => props.theme.colors.black300};
}
&:after {
color: ${(props) => props.theme.colors.primary};
}
`

const StyledImage = styled.img`
width: 24px;
margin-right: 1rem;
`
import {
Flex,
Image,
LinkBox,
LinkOverlay,
Table,
Tbody,
Td,
Th,
Thead,
Tr,
useToken,
} from "@chakra-ui/react"
import Link from "./Link"

export interface TableRow {
name: string
Expand All @@ -125,49 +31,111 @@ export interface IProps {
}

// TODO generalize this component - currently tailored for stablecoin market caps
const SimpleTable: React.FC<IProps> = ({ columns, content, hasError }) => (
<Table>
<Header>
{columns.map((column, idx) => (
<HeaderCell key={idx}>{column}</HeaderCell>
))}
</Header>
{hasError && (
<Row>
<Cell>
<Translation id="page-stablecoins-table-error" />
</Cell>
</Row>
)}
{!hasError && content.length === 0 && (
<Row>
<Cell>
<Translation id="page-stablecoins-table-loading" />
</Cell>
</Row>
)}
{content.map((row, idx) => {
const { name, marketCap, image, type, url } = row
const rowContent = (
<>
<Cell>
{image && <StyledImage src={image} alt="" />}
{name}
</Cell>
<Cell>{marketCap}</Cell>
<Cell>{type}</Cell>
</>
)
const SimpleTable: React.FC<IProps> = ({ columns, content, hasError }) => {
const [textColor] = useToken("colors", ["text"])

return (
<Table
variant="unstyled"
my={8}
borderRadius="sm"
border={`1px solid ${textColor}`}
bg="background"
mb={8}
minW="720px"
>
<Thead bg="ednBackground" color="text200">
<Tr boxShadow={`0 1px 1px ${textColor}`} mb="1px">
{columns.map((column, idx) => (
<Th
key={idx}
py={5}
fontSize="md"
fontWeight="normal"
letterSpacing="normal"
>
{column}
</Th>
))}

return url ? (
<LinkRow key={idx} to={url}>
{rowContent}
</LinkRow>
) : (
<Row key={idx}>{rowContent}</Row>
)
})}
</Table>
)
{content && content[0]?.url && (
<Th p={5} fontSize="md" fontWeight="normal" textAlign="end">
↗
</Th>
)}
</Tr>
</Thead>
<Tbody>
{hasError && (
<Tr p={4}>
<Td colSpan={4}>
<Translation id="page-stablecoins-table-error" />
</Td>
</Tr>
)}
{!hasError && content.length === 0 && (
<Tr p={4}>
<Td colSpan={4}>
<Translation id="page-stablecoins-table-loading" />
</Td>
</Tr>
)}

{content.map(({ name, marketCap, image, type, url }, idx) => (
<LinkBox
as={Tr}
display="table-row"
href={url}
key={idx}
color="text"
textDecoration="none"
boxShadow={`0 1px 1px ${textColor}`}
_hover={{
textDecoration: "none",
boxShadow: `0 0 1px ${textColor}`,
bg: "primary200",
color: "black300",
cursor: url ? "pointer" : "default",
}}
_focus={{
boxShadow: `0 0 1px ${textColor}`,
bg: "primary200",
color: "black300",
}}
>
amit-ksh marked this conversation as resolved.
Show resolved Hide resolved
<Td>
<Flex align='center'>
{image && <Image src={image} alt="" mr={4} boxSize={6} />}
{url ? (
<LinkOverlay
as={Link}
hideArrow
to={url}
color="inherit"
textDecoration="inherit"
_hover={{
textDecoration: "inherit",
}}
isExternal
>
{name}
Copy link
Contributor

@TylerAPfledderer TylerAPfledderer Dec 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is a personal recommendation, as it is a minor addition for A11y. (For @pettinarip approval 😄 )

Include visually hidden text for the LinkOverlay render only which reads (Opens a new window) (with the parens) since all the link that are ever passed into this table are assumed to be external and the screen-reader user should be notified of that.

Maybe should have this under a translation function too and supplied in the intl/en/common.json file

Suggested change
{name}
{name}
<VisuallyHidden>
<Translation id="sr-link-opens-new-window" />
</VisuallyHidden>

And the json add:
"sr-link-opens-new-window": "(Opens a new window)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @TylerAPfledderer, should I make these changes or wait for @pettinarip approval?

Copy link
Contributor

Choose a reason for hiding this comment

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

@amit-ksh I would wait to make sure it's OK since it is adding to the structure.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a worthy discussion we can have with the rest of the team on a different issue since I think that if we apply that change we should do the same with the rest of the external links we have on the site, to keep consistency. For that reason, I don't see that change as part of the scope of this PR/migration.

Happy to see this issue created and discussed with more people. I haven't seen this pattern a lot on other sites. I'm curious to hear what others think about it.

</LinkOverlay>
) : (
<>{name}</>
)}
</Flex>
</Td>
<Td><Flex align='center'>{marketCap}</Flex></Td>
<Td><Flex align='center'>{type}</Flex></Td>
{url && (
<Td color="primary">
<Flex align='center' justify='flex-end'>↗</Flex>
</Td>
)}
</LinkBox>
))}
</Tbody>
</Table>
)
}
export default SimpleTable