tinkerhubco / docnest-desktop

DocNest platform desktop app
2 stars 0 forks source link

UI | Components Stories #8

Closed isotopeee closed 5 years ago

isotopeee commented 6 years ago

Avatar - Avatar

Button - Button

Card - Card

Divider - Divider

Paper - Paper

Form - Wrapped in Formik

FormField - TextField | Checkbox | Radio

Grid - Grid

Icon - Icon

Menu - Menu

MenuItem - MenuItem

Progress - Progress

Snackbar - Snackbar

Stepper - Stepper

Table - Table

Typography - Typography

isotopeee commented 6 years ago

Here's the initial list of components that we'll need. Feel free to add up. I didn't include the global (such as Header and Leftsidebar) components since it should belong to the app layout domain.

isotopeee commented 6 years ago

Working on the Card component

isotopeee commented 6 years ago

I've pushed the initial changes I've made to feature/component-stories branch. I forgot to add the styled-components in the initial setup so I've added it to the branch as well.

jmaicaaan commented 6 years ago

OK, thank you for the changes. Let me work on other things. I will post what I will do so we can avoid conflicts or duplicate work. 👌

jmaicaaan commented 6 years ago

Working on the TextField & Password...

jmaicaaan commented 6 years ago

Please see my changes for Button Fab & Upload. Then for the TextField text & password ^^

isotopeee commented 6 years ago

Just for clarification, the Paper component will be a presentational component, and the Form component will be a helper component wrapped in formik Form just like the Field I'm talking about.

isotopeee commented 6 years ago

Also, I think we should prefer stateless functional components over class components for simplicity. It can be easily converted into a class component anyway.

Summary, stateless functional components for presentational components and class component for smart/container components.

Also, you might be wondering why I'm using the 👴 legacy function keyword when creating functional components. You might want to look read this article why I'm doing so.

jmaicaaan commented 6 years ago

Got it.

For the function usage, the article is just saying about the debugging or a much easier to find errors since function isn't an anonymous function. Besides from that, is there any advantages?

isotopeee commented 6 years ago

Aside from that, I don't think it has other benefits. But I think we're better off with one added benefit.

jmaicaaan commented 6 years ago

Okay thank you 👍

jmaicaaan commented 6 years ago

I've updated the files. Please see the changes

jmaicaaan commented 6 years ago

Please see changes for <Select /> formfield

jmaicaaan commented 6 years ago

For the FormField pickers, MaterialUI doesn't support an enrich UI/UX in this one. They only provide native designs. Can we use this https://material-ui-pickers.firebaseapp.com/usage? Or do you have any on the pocket?

jmaicaaan commented 6 years ago

Currently working with the snackbars. I left off some "time consuming" components for later 😝 #timeManagement #timeWise lol

jmaicaaan commented 6 years ago

<Snackbar /> and <Paper /> done with stories

isotopeee commented 6 years ago

Great Work! Fixing the snapshot tests

jmaicaaan commented 6 years ago

<Divider /> done with story

jmaicaaan commented 6 years ago

<Table /> done with story. Please see changes

jmaicaaan commented 6 years ago

Just an update, I'm still working with the <Form /> component

jmaicaaan commented 6 years ago

<Stepper /> done with story <Grid /> is done with story <Checkbox /> is done with story <CircularProgress /> is done with story <LinearProgress /> is done with story

jmaicaaan commented 6 years ago

Hi, I'm saving myself from breaking the DRY principle in regards to <Form /> and <Table />.

Why?

Render prop allow you to let the consumer of the component determine how to place their child elements. How to nest their elements according to their OWN WILL. I agree that is kind of flexible in terms of you want to have a "unchecked" elements under your component (Table -> TableBody -> TableRow -> TableCell) rendered. What more we can get besides from letting the consumer do it themselves? Enlighten me more.

Then why am I not doing the render prop pattern?

I don't know yet the use case where to use the render prop. I'm still grasping that react knowledge. 🙄 But one thing is I'm sure, I can see in that creating render prop breaks the DRY principle. Which is Don't Repeat Yourself.

I created the <Form /> is such a way to provide a clean interface usage. In such a way, "give me a schema and I will render it" rather than "give me a schema and render your elements below under my render prop please". In that way, we can make sure of the elements under form.

There are some cases, dynamic elements is not ideal as it CAN and POTENTIALLY provide inconsistencies.. Why? Imagine you have a <ABC /> component that is accepting a render prop

<ABC render={() => <button>Hello hi</button>}

and I want to reuse ` in to another file

<ABC render={() => <MaterialButton>Hi hello</MaterialButton>}

These things CAN and potentially break the inconsistencies across different sections of the app.

Final breaker If you have a lot of pages that needs a <Form /> like 5 pages.. Is it much more easier to just create a schema and don't worry about the elements that you will add? If there's a render prop, I will probably just copy and paste from different pages the elements then modify. Can we not make it better? We know what would be the "look" of our form. We have comps.

One thing, schema based is I don't like is the dynamic forms. It is kind of doing a lot more. But when it comes to static forms, it is good.

Every abstraction comes with a cost. With that I provide you a proposal form schema.

I believe it is a good value & a good ROI.

Let me hear your thoughts.

jmaicaaan commented 6 years ago

Btw, most of the components are already done with stories. Except for the pickers. I provided a link above if you can look at it, we can use that. Material-ui doesn't provide a "good" datepicker as they are pursuing the native look.

Besides from that, everything is already on the branch.

isotopeee commented 6 years ago

We can provide a default render prop such that we don't break the DRY principle. Usually, components using the render props pattern are generic components (List, Table, Form, etc.) therefore, we won't use it on our screens directly. They will serve as the base for building the other components (SignInForm, SignUpForm, NewPatientForm).

✅ We didn't break the DRY principle ✅ Standard look ✅ Provides consistency across different usage (Form will act as a generic component for building out other Forms. This applies to other generic components as well) ✅ Good on supporting dynamic forms

isotopeee commented 6 years ago

The material-ui-pickers looks great 🔥

isotopeee commented 6 years ago

I think we shouldn't wrap the components with styled because styled.extend will be deprecated. My bad ✌️

jmaicaaan commented 6 years ago

I have added material-ui-pickers as our dependency. You might got problem installing via yarn. Be sure you update your local dependency.

Also, I have added date-fns to be used by material-ui-pickers. In the documentation of material-ui-pickers it is suggesting to install currently at version @2.0.0-alpha.16 because there is a bug from the date-fns. See this issue for reference.

<DatePicker /> done with stories <TimePicker/> done with stories <DateTimePicker/> done with stories

The pickers needed the icons to be loaded. See this installation. Can we load the material icons from node_modules instead using html?

I didn't add the inline ability yet. But if we really need it we can easily add it too. Currently it is on another module, so I skipped it first and focus on getting the pickers up by default.

Let me know what we are still missing here. I'm looking forward to close this issue over the weekend and so.

isotopeee commented 5 years ago

It seems that the Table -> Cell -> Date stories broke (📸 snapshots in action) when date-fns was updated. Will try to look on the following issues:

https://github.com/date-fns/date-fns/issues/803 https://github.com/date-fns/date-fns/issues/799 https://github.com/date-fns/date-fns/issues/786

isotopeee commented 5 years ago

By the way, it's a good practice to run the test whenever we make changes to the components.

npm npm run test or npm run test-watch

yarn yarn test or yarn test-watch

Currently, there are 5 failing tests caused by this issue. I'll address that before closing this issue.

jmaicaaan commented 5 years ago

Oops sorry for that. Will also take a look on that! Havent run the snapshot tests 🙏

jmaicaaan commented 5 years ago

Looks like there's a breaking change on the formatting based from the links you've given. I remember I was looking directly at their latest documentation 😂😂😂

jmaicaaan commented 5 years ago

Btw you've finished working on it? Just saw your commits 👐

isotopeee commented 5 years ago

I've already fixed the issue regarding the change in date-fns date formatting. Will work on the material-icon font for material-ui-picker later.

isotopeee commented 5 years ago

🏁 Just finished adding the material-icons. Will fix the failing tests and review the components.

isotopeee commented 5 years ago

I've excluded the Menu and Snackbar from storyshots snapshot tests. It's taking me too much time to fix the tests issue. Check out this issue if you want to give it a shot.

We're almost done with this issue, I'll just review the components implementaton.

jmaicaaan commented 5 years ago

We can put it in the backlog. Nothing so serious on those components.

Yahoo, just a few steps and we're now done with this one. What a milestone 🔥