Darien Classrooms & Assignments Mk2 - Part 1 #107

shaunanoordin · 2017-12-05T18:13:55Z

PR Overview

This PR plans to improve the WildCam Darien Lab's Classrooms and Assignments.

Goals:

Only logged-in users can access Educators page (i.e. Classroom & Assignment management.) ( fixes Darien: secure classroom management #91 )
Non-logged-in users have access to a limited subset of pages and are prompted to login.
Educators can create & edit Darien-specific Classrooms.
Educators can create & edit Darien-specific Assignments.
[major][ambitious] Educators can select Subjects from the Map Explorer to create Assignments. (EDIT: maybe start with empty Assignments first?)

Expected BIG STUFF that needs handling:

Darien-specific Classroom & Assignment management will run head-long into the points discussed in Darien edit classroom view -- shared component reorganization? #85 , since I'll need to address the hardcoded I2A things in the common Classrooms components.

NOT in the scope:

Students pages. (i.e. "view my Assignments, view my Classrooms")

Dev Notes

Please note that I'm reorganising the way that the Darien components are organised, subdivided, and named.

Notably, I'm reconceptualising the "DarienRoutesContainer" container into the "DarienProgram" container, which contains more logic and (hopefully) is a better way of thinking of how we organise our Programs.

Status

WIP

shaunanoordin · 2017-12-07T14:07:54Z

PR Update

I think I'll split the planned updates for this PR into multiple parts. In this PR:

Only logged-in users can access Educators page (i.e. Classroom & Assignment management.) ( fixes Darien: secure classroom management #91 )
Non-logged-in users have access to a limited subset of pages and are prompted to login.
- New Darien 401 and 404 pages introduced.
- The Map (Explorer) page is still open to all.
  - NOTE: I'm separating 'Students' from 'Explorers', which will be a change from WildCam Gorongosa. 'Students' pages will come way later.
Darien components re-organised; we now have a 'root' DarienProgram component for, well, handling the Program itself.
- The Darien Home page has also been updated; mostly, this is a minor restyle and content/text tweak.

Next PR:

Educators can create & edit Darien-specific Classrooms.
Educators can create & edit Darien-specific Assignments.

As previously noted in "Expected Big Stuff": I'll likely need to clone & spinoff the Classroom and Assignment components used in I2A into "WildCamClassroomLayout" components.

Status

Ready for review. 👌 This will be followed up with a PR Part 2.

srallen

I really only have non-blocking comments, some of which could be addressed in your part 2 PR. I'll hold off on merging so you can choose how you want to address them.

Organization is much better 👍 . The old routes container was a pretty WTF reaction to React Router v4's API and non-sensical.

404 and 401 pages work well. Perhaps we can think about sharing those between the I2A side of the app too?

srallen · 2017-12-08T11:45:02Z

src/components/darien/Darien401.jsx

+import PropTypes from 'prop-types';
+import GenericStatusPage from '../../components/common/GenericStatusPage';
+
+const Darien401 = (props) => {


I've been switching to writing functional components using declaration rather than expression: function MyComponent() {}. React will correctly use the function name as the component display name this way. For some reason, React won't use it if it's an expression. It really helps with debugging using the React Dev Tools to see the correct names.

srallen · 2017-12-08T11:45:35Z

src/components/darien/Darien404.jsx

+import PropTypes from 'prop-types';
+import GenericStatusPage from '../../components/common/GenericStatusPage';
+
+const Darien404 = (props) => {


Same comment about declaration vs expression for ease of debugging purposes.

srallen · 2017-12-08T11:45:50Z

src/components/darien/DarienEducators.jsx

+import PropTypes from 'prop-types';
+import ClassroomsLayout from '../classrooms/ClassroomsLayout';
+
+const DarienEducators = (props) => {


Same comment about declaration.

srallen · 2017-12-08T11:46:16Z

src/components/darien/DarienHome.jsx

-  constructor(props) {
-    super(props);
-  }
+const DarienHome = (props) => {


Same comment about using declaration.

srallen · 2017-12-08T11:47:00Z

src/components/darien/DarienMap.jsx

@@ -3,20 +3,20 @@ import PropTypes from 'prop-types';
 import MapExplorer from '../../containers/maps/MapExplorer';
 import mapConfig from '../../lib/wildcam-darien.mapConfig.js';

-const DarienExplorer = (props) => {
+const DarienMap = (props) => {


Same comment about using declaration.

srallen · 2017-12-08T11:48:31Z

src/containers/darien/DarienProgram.jsx

+import Darien404 from '../../components/darien/Darien404';
+import GenericStatusPage from '../../components/common/GenericStatusPage';
+
+class DarienProgram extends React.Component {


I think you can still use the higher order connect function with functional components, so I don't think this has to be a class, unless you have plans to add some lifecycle methods to handle something.

srallen · 2017-12-08T11:52:11Z

src/components/darien/DarienHome.jsx

+    <ProgramHome className="darien-home">
+      <Hero
+        className="program-home__hero"
+        background={<Image src="https://placeimg.com/1000/1000/nature/any" fit="cover" />}


The Darien team might want to pick an actual background image here rather than use this placeholder. Maybe something from the project?

I put the string for the background source into the program metadata for I2A just so it's consistent with referencing everything about the program from the database. Just so you know, the controllers for update hasn't been done for programs on the API, so you have to ask for someone update it via the Rails console.

srallen · 2017-12-08T11:55:21Z

src/components/darien/DarienHome.jsx

+        <Box align="center" direction="column">
+          <Paragraph>Investigate ecological questions by exploring trail camera data using an interactive map. Filter and download data to perform analyses and test hypotheses.</Paragraph>
+
+          <Box pad="medium" size="medium">


Just a minor styling comment, but perhaps the Box should be defined as a bigger size to allow for the paragraphs to be displaying inline? Even with a fairly high resolution screen, I still have to scroll to see both options for the lab to get started.

srallen · 2017-12-08T11:57:39Z

src/components/darien/DarienHome.jsx

+
+          <Box pad="medium" size="medium">
+            <Paragraph> Alternatively, you can simply explore the data.</Paragraph>
+            <Button path="/wildcam-darien-lab/map/" label="Explorer" />


Maybe add className="button--secondary" here so this button correctly gets hover/focus styles? I still have had a moment to figure out why Grommet is being inconsistent with the styles.

srallen · 2017-12-08T11:58:32Z

src/components/darien/DarienHome.jsx

+          <Box pad="medium" size="medium">
+            <Paragraph>If you are an educator, you can set up private classrooms and invite your students to join. Curate data sets or let your students explore on their own. Guided activities and supporting educational resources are also available. {(signedIn) ? null : '(Sign In required)'}</Paragraph>
+            {(signedIn)
+              ? <Button path="/wildcam-darien-lab/educators/" label="Educator" />


Maybe add className="button--secondary" here too?

shaunanoordin · 2017-12-08T14:41:50Z

Thanks @srallen, that's some great feedback!

I'll perform the following updates for this PR:

Darien401 and Darien404 will now become common/Status401, common/Status404
const myComponent = () => {} will be transformed into function myCompontent() {}
className='button--secondary' will be added to the Educator and Explorer buttons

I won't touch the styling (background image, Box sizes) for WildCam Darien this round though, as I'll need to talk to HHMI about these things anyway, so I expect them to be changed soon-ish.

shaunanoordin · 2017-12-08T14:44:45Z

Actually, dangit, now that I look at it, what WildCam Darien home page styling can be improved quite a bit. Mrrrgh!

shaunanoordin · 2017-12-08T15:34:03Z

PR Update

This PR has been updated to reflect the excellent feedback from @srallen

We now have common/Status401 and common/Status404 pages.
Darien Component definitions now use declarations(), (not) => { expressions }.
Darien Home page buttons use proper button stylings.
Darien Home page... RESTYLED! For now! I'll need to get HHMI to review the text & etc but it's an improvement.

Status

Ready for re-review! 👍

srallen

I have a couple of minor comments on naming, but this otherwise looks good to merge!

srallen · 2017-12-08T16:04:44Z

src/components/common/Status401.jsx

+import PropTypes from 'prop-types';
+import GenericStatusPage from './GenericStatusPage';
+
+function Darien401(props) {


Maybe name the function Status401?

srallen · 2017-12-08T16:05:01Z

src/components/common/Status404.jsx

+import PropTypes from 'prop-types';
+import GenericStatusPage from './GenericStatusPage';
+
+function Darien404(props) {


Name this Status404?

shaunanoordin · 2017-12-08T16:14:13Z

Nyarp! 😱 Sorry, I forgot to rename Darien401/404's actual function names - I'll take care of that in part 2. Thanks for spotting that!

shaunanoordin added 3 commits December 5, 2017 17:44

DarienProgram rearrangement

ae2921a

Removed unnecessary hash

a916dbb

Add basic sign-in checks

65d587a

shaunanoordin added the work in progress label Dec 5, 2017

shaunanoordin added 3 commits December 6, 2017 17:48

Add 401 and 404 pages for Darien

e378209

Add basic Darien Classrooms

508f5ab

Restyled Darien home page

d11f541

shaunanoordin assigned srallen Dec 7, 2017

shaunanoordin removed the work in progress label Dec 7, 2017

shaunanoordin changed the title ~~Darien Classrooms & Assignments Mk2~~ Darien Classrooms & Assignments Mk2 - Part 1 Dec 7, 2017

srallen reviewed Dec 8, 2017

View reviewed changes

shaunanoordin added 4 commits December 8, 2017 14:50

Darien components definitions changed from expression to declaration

6103fa2

button styles updated

e6ca190

darien/Darien401 and 404 changed to common/Darien401 and 404

2623ef4

Prettied up the Darien Home page

98949ef

srallen approved these changes Dec 8, 2017

View reviewed changes

srallen merged commit f2a4f61 into master Dec 8, 2017

shaunanoordin mentioned this pull request Dec 20, 2017

Darien Classrooms & Assignments Mk2 - Part 2 #114

Closed

4 tasks

shaunanoordin mentioned this pull request May 11, 2018

WildCam Lab Classrooms - 2018 Rebuild - part 1 #122

Merged

shaunanoordin deleted the darien-classrooms branch July 2, 2018 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Darien Classrooms & Assignments Mk2 - Part 1 #107

Darien Classrooms & Assignments Mk2 - Part 1 #107

shaunanoordin commented Dec 5, 2017

shaunanoordin commented Dec 7, 2017

srallen left a comment

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

srallen Dec 8, 2017

shaunanoordin commented Dec 8, 2017 •

edited

Loading

shaunanoordin commented Dec 8, 2017

shaunanoordin commented Dec 8, 2017

srallen left a comment

srallen Dec 8, 2017

srallen Dec 8, 2017

shaunanoordin commented Dec 8, 2017

Darien Classrooms & Assignments Mk2 - Part 1 #107

Darien Classrooms & Assignments Mk2 - Part 1 #107

Conversation

shaunanoordin commented Dec 5, 2017

PR Overview

Dev Notes

Status

shaunanoordin commented Dec 7, 2017

PR Update

Status

srallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaunanoordin commented Dec 8, 2017 • edited Loading

shaunanoordin commented Dec 8, 2017

shaunanoordin commented Dec 8, 2017

PR Update

Status

srallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaunanoordin commented Dec 8, 2017

shaunanoordin commented Dec 8, 2017 •

edited

Loading