Skip to content

Changed TextField label font size to prevent overflowing parent #108

Merged
merged 3 commits into from
Nov 4, 2020
Merged
Changes from 1 commit
Commits
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
20 changes: 18 additions & 2 deletions src/components/ItemTableFilter/ItemTableFilter.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,35 @@
import React from 'react';
import PropTypes from 'prop-types';
import { TextField } from "@material-ui/core";
import { TextField, makeStyles } from "@material-ui/core";
import { pink, purple } from '@material-ui/core/colors';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these colors needed? They've been imported but aren't used.


export default function ItemTableFilter({ label, onChange }) {

const useStyles = makeStyles({

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove unnecessary spacing.

labelRoot: {
fontSize: 14,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't arbitrarily change font sizes. Its a nightmare for accessibility and constancy. We'll need a solution that scales with whatever content we put into the filter rows.

},

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove unnecessary spacing.

},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Combine these block closers )};

);

const classes = useStyles();

return (
<>
Copy link
Collaborator

Choose a reason for hiding this comment

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

React fragments <> and </> are only needed if there is more than one root element being returned from a component. Because you're only using the TextField component here, you don't need fragments. Please remove them.


<TextField
label={label}
onChange={onChange}
classes={{ root: classes.inputLabelStyles }}
InputLabelProps={{
classes: { root: classes.labelRoot, focused: classes.labelFocused }}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

classes.labelFocused doesn't exist. Should it be created in useStyles() or removed from this classes prop?

color="secondary"
type="search"
size="small"
variant="outlined"
fullWidth

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove unnecessary spacing.

/>
</>
);
Expand Down