Skip to content
This repository has been archived by the owner on Jan 28, 2022. It is now read-only.

WIP: Convert the UI to be React based #1

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

WIP: Convert the UI to be React based #1

wants to merge 26 commits into from

Conversation

wbushey
Copy link
Member

@wbushey wbushey commented Sep 28, 2016

No description provided.

@jhsu802701
Copy link
Collaborator

I'd like to provide input, but I need to get up to speed on a few things. I'm not familiar with React. Why do we want a React-based UI? What does it replace, and why is React better? How should the conflict in app/views/layouts/application.html.haml be resolved? In addition to running all of the tests and making sure that they pass, is there anything else I need to check?

SIDE NOTE: The large number of changes makes this pull request intimidating to review. I prefer to make numerous small changes instead of one big one. Making too many changes all at once makes troubleshooting much more difficult if something doesn't pan out.

@wbushey
Copy link
Member Author

wbushey commented Nov 14, 2016

Good questions @jhsu802701. The whats and whys are covered a bit by the description of the Rebuild UI in React project (https://github.com/OpenTwinCities/adopt-a-tree/projects/2 and click 'Details'). Basically, this project aims to re-write the UI to be much more testable and modular, with the ultimate goal of making future UI development easier.

As far as testing, this branch includes JavaScript unit tests in spec/javascripts. At the moment, running these in the command line will error out, but they can be run in browser by starting the rails server as normal and visiting <server_address>/specs .

As a rule of thumb, I definitely agree with you that MRs should be much smaller. Once work started on this branch, it became clear that this project is a pretty significant undertaking, and that it won't make sense to merge it into master until quite a bit of work has been done. So, we decided to treat this branch as a long term project branch. Folks wishing to work on this project should create merge requests against this branch (react), not master. When this branch is ready to be deployed, we'll then merge it into master.

Another good question regarding conflicts. At the moment, I'm not sure (I know, that's a bad answer). I just rebased this branch with master and addressed a couple of conflicts in doing that; I'm actually a little surprised that GitHub is still showing conflicts.

wbushey and others added 24 commits December 11, 2016 11:19
Finished display of Field
Readding connection pool dependency
DRYed up form partials
Added a couple of missing translations
Fields pass on required and private attributes
A few display tweaks
Rendering Edit Profile via React
Moved most map related JS into a ThingMap class
Add spec helper to mock out Google Maps library during tests
* All field components accept a callback function for state change
* Addressing warnings about keys and proptypes
Fixes #33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants