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

New classroom form #55

Merged
merged 4 commits into from
Feb 25, 2016
Merged

New classroom form #55

merged 4 commits into from
Feb 25, 2016

Conversation

simoneduca
Copy link
Contributor

No description provided.

@simoneduca simoneduca added this to the Alpha milestone Feb 24, 2016
Enable navigation with browserHistory.
Remove some logs.
@shaunanoordin
Copy link
Member

Code looks legit and the functionality of the Add New Classroom form works! I only have a few queries before I merge:

Q1: Just to confirm, react-router was upgraded to v2.0.0 for the browserHistory functionality, right?
Q2: The (optional) 'Subject' field of a class, after being set during the Add New Classroom process, isn't displayed anywhere. I assume this is a minor functionality that's coming in a later feature branch?

Notes:

  • Unintended behaviour can be created by logging in & going to the Classrooms page, then clicking the Logout... because the page still LOOKS like you're logged in, allowing you to access Classroom and Add New Classroom features. (Although any attempt to actually add a new classroom while not logged in will fail silently)
  • However, this is NOT a blocking factor for this branch - this is more related to Issue Teacher / student routes need to be authenticated #26: Authentication priorities.

@simoneduca
Copy link
Contributor Author

@shaunanoordin 1) react-router has been upgraded because some feats of the older API will stop being supported/are already deprecated, e.g. mixins, history.transitionTo, etc.
2) correct
Notes should be taken care of when routes are properly authenticated.

@shaunanoordin
Copy link
Member

  • Code sanity check passed
  • Add New Classroom functionality verified.

PR Status: Validated
Action: Merging

shaunanoordin added a commit that referenced this pull request Feb 25, 2016
@shaunanoordin shaunanoordin merged commit 3b55861 into master Feb 25, 2016
@simoneduca simoneduca deleted the new-form branch February 25, 2016 20:14
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.

2 participants