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.

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

Prettify Admin-UI #3

Closed
wants to merge 2 commits into from
Closed

Prettify Admin-UI #3

wants to merge 2 commits into from

Conversation

juni-vogt
Copy link

Made the Admin-UI prettier by adding some Material-UI components 🌈

@juni-vogt juni-vogt changed the title Matthias Prettify Admin-UI Oct 1, 2018
@remarcable remarcable closed this Oct 9, 2018
Copy link
Owner

@remarcable remarcable left a comment

Choose a reason for hiding this comment

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

Danke dir, Matthias!

</div>
<p>
{`${candidateNumber ? `Kandidat Nr. ${candidateNumber} ` : ''}${name}`}
<EditDialog key={_id}>
Copy link
Owner

Choose a reason for hiding this comment

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

Den EditDialog n mal zu rendern würde ich nicht machen – du willst ja zu keinem Zeitpunkt zwei candidates gleichzeitig bearbeiten sondern immer nur einen. Also brauchst du auch nur einen Dialog der sich um den Rest kümmert

import { Link } from 'react-router-dom';

// material-ui
import { withStyles, MuiThemeProvider, createMuiTheme } from '@material-ui/core/styles';
Copy link
Owner

Choose a reason for hiding this comment

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

Um unnecessary imports und co aufzudecken wäre es sinnvoll, einen Linter und prettier zu benutzen :)

UserList
</Button>

<Button
Copy link
Owner

Choose a reason for hiding this comment

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

Bei den Buttons ist super viel duplication => eine kleine Button component erstellen und dort nur die Infos reinpassen, die sich unterscheiden


const styles = theme => ({
margin: {
margin: theme.spacing.unit,
Copy link
Owner

Choose a reason for hiding this comment

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

Wie gesagt, das hier (Magic Numbers reduzieren) ist mega nice. Hab ich bisher nicht gemacht ._.

@@ -18,6 +19,10 @@ import sortFullShowGames from '/imports/api/helpers/sortFullShowGames';
import AdminLayout from '/imports/ui/Layouts/AdminLayout';
import UpdateGames from './UpdateGames';

// material-ui
Copy link
Owner

Choose a reason for hiding this comment

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

sollte man nicht brauchen, sieht man ja am import.
Wie wenn man so etwas tut:

// when x equals 5
if(x === 5) {...}

);
const ShowScreen = ({ classes, isReady, interactions, games, hasNext, hasPrevious }) => {

return (
Copy link
Owner

Choose a reason for hiding this comment

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

Warum zusätzlich ein return?


ShowScreen.propTypes = propTypes;

export default withTracker(() => {
export default withStyles(styles)(withTracker(() => {
Copy link
Owner

Choose a reason for hiding this comment

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

Funktioniert das sorum? Bei mir hat das immer einen Fehler geworfen. Lösung war, das withStyles(styles)(Component) in die returnte Funktion von withTracker zu übergeben

@@ -9,4 +9,7 @@ export const theme = createMuiTheme({
primary: orange,
secondary: blue,
},
spacing: {
unit: 16
Copy link
Owner

Choose a reason for hiding this comment

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

lieber dort, wo es nötig ist, theme.spacing.unit * 2 verwenden, da das hier quasi "global" ist

@@ -4,6 +4,22 @@ import PropTypes from 'prop-types';
import SimpleSchema from 'simpl-schema';
import AutoForm from 'uniforms-material/AutoForm';

// material-ui
import { withStyles, MuiThemeProvider, createMuiTheme } from '@material-ui/core/styles';
Copy link
Owner

Choose a reason for hiding this comment

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

most of it sollte nicht notwendig sein.


<AutoForm
schema={schema}
onSubmit={(data) => updateCandidate({ _id, ...data })}
Copy link
Owner

Choose a reason for hiding this comment

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

EditDialog wird nicht geclosed beim submitten

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants