Redux Stores and componentize functionality

mogoodrich · June 6, 2018, 1:26pm

One other point I meant to mention:

We are definitely looking for advice/best practice on how to organize the directory the structure… right now we broke everything out in directory by type, ie “reducers”, “sagas”, etc, but we are wondering if we should group things more by functional areas… give now we have “loginApi.js”, “loginReducers.js” and “loginSagas.js” spread across three different folders (and then Login components in another one).

larrystone · June 6, 2018, 4:17pm

No, the accordion didn’t work. I couldn’t get the existing implementation to work at all because the project I was testing it on was not making use of redux at all. I had to comment out the sagas, action creators and other redux related codes, pack and install on the project to successfully test the accordion.

I would suggest we have a workflow that allows the needed data/actions for the components to be passable as props. That way, the components only renders data. To take it further, we may then decide to have the component check the presence of this props. If they are not present, then the component should go on to make the necessary API calls to fetch those data (within it’s component state). This approach is so minimalistic that it reduces the need for many external dependencies or setup to use the module.

It was quite challenging to test. I will raise a PR before the end of the day. I will also try to update the readme to provide more information on how to locally test these components.

The current directory structure looks good to me. I only made a few adjustment when doing the accordion component. I will also think the api files could be moved into the actions folder.

Using redux thunk and saga together might be harmless but I am not sure developers (especially lazy one like me ) would want to do that.

Yes, you are right, context API is not here to replace thunk and saga, my bad.

mogoodrich · June 6, 2018, 7:51pm

Not sure I understand or agree with this 100%, but would be interested in an example of how this will work. Would the business logic/API be bundled with the component?

The more I work with this, the more I’m convinced we are going to want to go with a “duck” structure as opposed to a functional structure. In fact, I think I will try to take a stab at this refactoring in the next couple of days.

I’m looking forward to your accordion pull request!

Take care, Mark

fyi @cioan @mseaton

ssmusoke · June 6, 2018, 9:40pm

All the JS stuff is way above my head (apologies if I do not make sense), if we are using Boostrap 4.x as the foundation for the OpenMRS UI components, etc should we not be leveraging what already exists Collapse · Bootstrap?

larrystone · June 7, 2018, 1:06am

Now, that’s another tricky topic to discuss, bootstrap uses jQuery and jQuery is not recommended on react projects.

Yes it will. I will try and work on an implementation of this thought before the end of the week.

ssmusoke · June 7, 2018, 11:04am

@larrystone if we cannot use the JQuery at least we should be able to use the CSS styling … That way even the Angular and regular GSP components can benefit

larrystone · June 7, 2018, 12:28pm

Yes, I agree, we can use their css. Which brings me to question I and my team asked some months ago. When do we plan to start using bootstrap on openmrs and which version do we intend to adopt?

ssmusoke · June 7, 2018, 12:36pm

@larrystone The latest version 4.1 - and the start is with the current OWAs being worked on.

We cannot have a massive re-design to Boostrap at once, so will move any OWAs and new components being designed, so will be a hybrid for a while.

The design approach is called Strangler - see https://martinfowler.com/bliki/StranglerApplication.html

larrystone · June 7, 2018, 12:49pm

Great. That’s good

mogoodrich · June 7, 2018, 12:56pm

Cool… I would think we’d want to de-couple business logic from components rather than bundling them into the components, but I’m interested in seeing what you come up with.

I have actually included react-bootstrap in the openmrs-react-components project (looks like it targets Bootstrap 3):

I’ve used it currently here for display and layout elements (Grid, Row, Button, etc):

github.com

openmrs/openmrs-react-components/blob/master/src/components/login/LoginForm.jsx#L4


      
          import React from 'react';
          import PropTypes from 'prop-types';
          import { connect } from 'react-redux';
          import { Field, reduxForm, formValueSelector } from 'redux-form';
          import FieldInput from '../widgets/FieldInput';
          import Errors from '../errors/Errors';
          import '../../../assets/css/loginForm.css';
          import { Button } from 'react-bootstrap';
          
          
          let LoginForm = props => {
            const { handleSubmit, pristine, reset, submitting, locations, isFormComplete } = props;

One interesting thing is that the CSS styling needs to be imported separately (see here). I actually didn’t bundle the css files within openmrs-react-components, but rather gave instructions on how developers can include them in apps that utilize the library. The idea was that this would give developers the option to determine which, if any, bootstrap css files to include.

Take care, Mark

mogoodrich · June 7, 2018, 4:42pm

@larrystone I merged in the Accordion changes making one small tweak (changing the name). Thanks!

I did test and I was able to use it in a module without having to wire in all the reducers and sagas, which is what I would have hoped since it doesn’t need to use and of the reducers or sagas. My way of testing this was to create a new “create-react-app” project and importing the Accordion from @openmrs/react-components. To get it to work, the only additional thing I had to do was configure babel-polyfill (following the instructions here). Now, since babel-polyfill is only required for sagas (I believe) I would like it better if a consumer did not have to include this if they aren’t using sagas… so let’s continue to think about how we can improve this, but I’m fine with this requirement for now. I’m not quite sure why you had to comment out all the sagas, etc?

Take care, Mark

mogoodrich · June 7, 2018, 4:45pm

fyi, I plan to do some major refactoring into more of a “duck” structure, hopefully today… I’ll sent out an alert (and suggest other developers pull changes ASAP) once I do this.

larrystone · June 7, 2018, 9:19pm

@mogoodrich Thanks for the update. I will be on the look out for the updates.

mksd · July 17, 2018, 3:25pm

@mogoodrich what’s your status update on putting together a React OWA with Redux and all the tech stack that might go with it? I’m looking at starting an OWA for Attachments and I’m wondering what would be the best starting point to 1) be in line with community practises and 2) benefit of the “best” libraries/patterns… etc.

Cc @dkayiwa (in regards to all the work done with OE OWA).

mogoodrich · July 17, 2018, 6:18pm

@mksd we have been continuing to develop the openmrs-react-components library here:

https://github.com/openmrs/openmrs-react-components

Unfortuantely, we really have been only been building what we need for the current application we are building, but I’m trying to make sure we put as much re-usable functionality in openmrs-react-components rather than our application code. Also, as we are just learning React, I can’t claim that everything in react-components is best practices.

That being said, I highly encourage to look at and use openmrs-react-components and contribute any common functionality you build there. Even if it doesn’t end up as complete/polished/organized as we like, at least it will be in a central place. Please don’t hesitate to reach out if you see something that you think should be changed–especially if it’s something that blocking you from contributing. we will be actively working on openmrs-react-components for the next several months, I suspect.

Also, I had started building some form functionality into openmrs-react-components but recently removed it because I think we will be able to use Bahmni form-controls within our (non-Bahmni) PWA, which will be great (see: Using Bahmni Form-Controls)

Take care, Mark

mksd · July 18, 2018, 7:57am

Great @mogoodrich, thanks. I don’t see any reason why I would not use openmrs-react-components. So I’ll have a look at that and report back.

Could you point me to your PWA? I could use this as a starting point. So it depends on and uses openmrs-react-components right?

Also I like that idea of a PWA, but what are the implications within OpenMRS, does this load basically exactly the same way an OWA would load?

dkayiwa · July 18, 2018, 7:59am

Can you guys step in here with your experience? @Andela

mksd · July 18, 2018, 10:43am

I guess that’s the guy: openmrs-pwa-workflow?

Looks really interesting, thanks for pointing to the scaffolding tool in the README, super helpful. I’m sure I’ll have tons of questions.

mogoodrich · July 18, 2018, 1:30pm

Thanks @dkayiwa… yes, that’s it.

Yeah, most of the README is generated via the scaffolding tool, all except that first part we wrote, so I wanted to make that clear…

The way we have the PWA working right now is that it’s a completely separate application from OpenMRS… in our demo environments, we have Apache serving it directly, instead of bundling it through OpenMRS. For this app in particular, I wanted to really make something that was stand-alone… though the jury is still out on whether that was the correct approach… it introduces extra complexity such as figuring out how to configure the OpenMRS Rest endpoint that it should connect to, how to persist OpenMRS log-in sessions, etc… it’s certainly possible we may decide the OWA approach is better in the long run.

My advice would be to stick with an OpenMRS OWA… we still plan to use the OWA model for modules we want to bundle directly in our PIH EMR. You should be able to import the react-components library and use it in the same manner as we use in the PWA (and if there are issues, let me know, because we should fix them).

Take care, Mark