Skip to content
This repository has been archived by the owner on Mar 10, 2018. It is now read-only.

Incorporate Redux Router #8

Merged
merged 12 commits into from Oct 19, 2015
Merged

Incorporate Redux Router #8

merged 12 commits into from Oct 19, 2015

Conversation

doomspork
Copy link
Owner

This is a WIP.

@doomspork
Copy link
Owner Author

@paultannenbaum this still needs lovin' but it's working! 🎉 It loads the health checks, polls for updates, and keeps the UI up-to-date. If you wanted to run it locally let me know and I can give the run down.


export function deleteHealthCheck(id, csrf_token) {
return function(dispatch) {
request.del('/health_checks/' + id)
Copy link
Contributor

Choose a reason for hiding this comment

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

ES6 has string interpolation, so you shoudl be able to make this look like: request.del('/health_checks/${id}')

*** the string interpolation requires back ticks instead of a single quote, but I didn't know how to escape it in the markdown ***

@paultannenbaum
Copy link
Contributor

Looks pretty good to me. The only part that looks like it might need to be broken up is the chart summary, but it is fine for now. We should get this merged in and then do a cleanup (add linting, do some html/css refactoring, etc.) and it will be a lot cleaner.

@doomspork
Copy link
Owner Author

This is still very much a work in progress, I haven't yet pushed my last few days of work. Let me finish up the auth and I'll get those changes pushed up for review + merge.

@doomspork
Copy link
Owner Author

Alright @paultannenbaum have at it.

I'm not super concerned about trivial syntax/styleguide errors right now, let's focus on the organization and functionality. Once we have everything done and working we can come back through and polish.

I'm going write up some instructions for you on how to run both Hospital and Medic.

else
conn
|> put_flash(:info, "Bad login.")
|> render("new.html", changeset: changeset)
|> put_status(401)
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm going to DRY up this JSON error handling later.

@doomspork
Copy link
Owner Author

@paultannenbaum instructions can be found at: https://gist.github.com/doomspork/f3f64eceab9c1ec82a14

@doomspork
Copy link
Owner Author

Probably outside the scope of this PR but login persisting on page refresh doesn't seem to work. It would be cool to figure that one out.

Panel content
</div>
</div>
<div id="root" csrf-token="<%= get_csrf_token() %>"></div>
Copy link
Owner Author

Choose a reason for hiding this comment

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

This page can be deleted.

@paultannenbaum
Copy link
Contributor

Looks good to me. Do have a couple questions in regards to the router, but everything else looks great. Lets get this merged in and then do some cleanup

@doomspork
Copy link
Owner Author

@paultannenbaum here are the two examples I based most of this on:

@doomspork doomspork changed the title Redux refactor Incorporate Redux Router Oct 19, 2015
@paultannenbaum
Copy link
Contributor

So I did some googling and from what I interpreted, it looks like we are both right :) The docs right now are using the react router without using action creators, but using route handling via actions is the correct approach and they are waiting on the docs to be updated. We can shelve this for now and just make it a point to refactor if you want to get this merged in.

References:
reduxjs/redux#177
reduxjs/redux#227

doomspork added a commit that referenced this pull request Oct 19, 2015
@doomspork doomspork merged commit 5697dc5 into master Oct 19, 2015
@doomspork doomspork deleted the redux-refactor branch October 19, 2015 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants