Update notes components #419

joshuasbates · 2018-06-15T20:50:21Z

Sorry for the large change set, but there were lots of changes needed to make this usable for a generic use case, but also add some flexibility for the custom ones (e.g., where notes had attachments).

…rops" This reverts commit 63dc209.

…assName to card; additional cleanup and testing

…h other note classes

joshuasbates

The changes to the components look good overall (one small comment there). The main feedback is in regard to the Stories.

joshuasbates · 2018-07-02T17:23:35Z

stories/Note.js

+        onDelete={action('onDelete')}
+        onEdit={action('onEdit')}
+        onSave={action('onSave')}
+        onUndelete={action('onUndelete')}
      >


Might be good to add the rows and saving knob here too.

joshuasbates · 2018-07-02T17:27:19Z

stories/Notes.js

-          onSave={n => alert(`Save Add: ${JSON.stringify(n)}`)}
+          onCancel={action('onCancel')}
+          onChange={action('onChange')}
+          onSave={action('onSave')}
        />}
        adding={adding}


The adding prop was removed by your recent changes, so this prop and the knob should be removed.

joshuasbates · 2018-07-02T17:27:38Z

stories/Notes.js

-          onSave={n => alert(`Save Add: ${JSON.stringify(n)}`)}
+          onCancel={action('onCancel')}
+          onChange={action('onChange')}
+          onSave={action('onSave')}
        />}
        adding={adding}


The adding prop was removed by your recent changes, so this prop and the knob should be removed.

joshuasbates · 2018-07-02T17:29:09Z

stories/Notes.js

@@ -78,27 +78,26 @@ storiesOf('Notes', module)
      <Notes
        addControl={<EditableNote


The addControl prop was removed by your recent changes, so should be removed from the example.

joshuasbates · 2018-07-02T17:31:46Z

stories/Notes.js

@@ -1,5 +1,5 @@
 import React from 'react';
-import { storiesOf } from '@storybook/react';
+import { action, storiesOf } from '@storybook/react';
 import { boolean, text } from '@storybook/addon-knobs';
 import { EditableNote, Note, Notes } from '../src';



The description below should have the last sentence (the one referring to addControl) removed.

joshuasbates · 2018-07-02T17:32:23Z

stories/Notes.js

@@ -43,20 +44,18 @@ storiesOf('Notes', module)
      <Notes
        addControl={<EditableNote


The addControl prop was removed by your recent changes, so should be removed from the example.

joshuasbates · 2018-07-02T17:37:06Z

stories/Notes.js

+              onDelete={action('onDelete')}
+              onEdit={action('onEdit')}
+              onSave={action('onSave')}
+              onUndelete={action('onUndelete')}


For some reason, when I view the Storybook, select the "deleted" knob, then click on the "undo" link in the DeletedNote view, it reroutes the page rather than recording the onUndelete action. Are you seeing this too?

joshuasbates · 2018-07-02T17:45:01Z

stories/Notes.js

@@ -78,27 +78,26 @@ storiesOf('Notes', module)
      <Notes


This example can be simplified. With children, we can remove all props that would have been forwarded to internally-created Note components. Since these props are explicitly passed to each Note in the child body, there's no need to pass them into Notes (now that it has no unique props itself), e.g.,

<Notes> {notes.map(note => ( <Note note={note} onCancel={action('onCancel')} onChange={action('onChange')} onDelete={action('onDelete')} onEdit={action('onEdit')} onSave={action('onSave')} onUndelete={action('onUndelete')} saving={note.saving} /> ))} </Notes>

(plus you can get rid of the <div> in the map body)

joshuasbates · 2018-07-02T17:51:28Z

src/components/NoteHeader.js

+        <div
+          className="d-inline-flex align-items-center"
+          ref="title"
+        >


The text-muted property seems to have been dropped. It appears to be a part of the Saffron standard.

In use: https://qa.qa.appfolio.com/maintenance/mobile?assignedUserId=any#/133920/notes

joshuasbates · 2018-07-02T20:37:02Z

stories/EditableNote.js

+    const saving = boolean('saving', false);
+
+    const note = withNote ? noteToEdit : { text: '' };
+    note.saving = saving;


Since saving is a component prop now, this should be removed and the knob should be added to the EditableNote inputs. (same for the next doc block)

sveinn · 2018-07-02T21:11:27Z

src/components/EditableNote.js

+            <Input
+              autoFocus
+              disabled={saving}
+              ref="text"


what is this ref for?

These were left over from the original impl. The unit test uses these refs to find elements. We should get away from that, IMHO, but I didn't go that far.

No worries, just curious

joshuasbates · 2018-07-02T21:14:02Z

src/components/EditableNote.js

@@ -35,7 +35,7 @@ class EditableNote extends React.Component {
    const { date, errors, text } = note;

    return (
-      <Card className={className}>
+      <Card outline color="info" className={className}>


Explanation of change: Editable note card should have white background; info border

joshuasbates · 2018-07-02T21:14:41Z

src/components/NoteHeader.js

-          className="d-inline-flex align-items-center"
-          ref="title"
-        >
+        <div className="d-inline-flex align-items-center text-muted">


Explanation of change: remove unused ref, add text-muted

sveinn · 2018-07-02T21:24:50Z

src/components/NoteHeader.js

+          <span className="m-0 my-1 mr-auto">
+            <span className="hidden-xs-down">
+              {edited ? 'Last edited' : 'Posted'}
+              {from ? <span className="js-note-header__from">{` by ${from}`}</span> : ' '} on <span className="js-note-header__date">{dateFormat(date, 'ddd, MMMM D, YYYY "at" h:mm A')}</span>


It's a bit odd to me that there are different date formats for Edited and non-edited notes, but that's the way it is in APM. Perhaps it makes sense to have these customizable. Not a big deal though

Just to be clear, the date format is the same, only the labeling of that date changes:

"Posted by Gary Thomas on Mon, July 2, 2018 at 2:33 PM"
vs.
"Last edited by Gary Thomas on Mon, July 2, 2018 at 2:33 PM"

I meant compared to short dateformat below on line 52: dateFormat(date, 'M/D/YY h:mm A') but I see now that's for small viewport widths.

…y, recommended practice)

gthomas-appfolio

👍

joshuasbates added 11 commits May 25, 2018 14:16

Revert "jb - Update BoundForm to allow update of object through new p…

567cfeb

…rops" This reverts commit 63dc209.

Merge branch 'master' of https://github.com/appfolio/react-gears

2921fea

Merge branch 'master' of https://github.com/appfolio/react-gears

eaf5781

jb - Make Notes help bubble text more customizable

cbdfeb5

jb - Render Notes as a block panel; add missing tests

15c8c25

jb - EditableNote: update callbacks to specify the note; propagate cl…

1ee453d

…assName to card; additional cleanup and testing

jb - EditableNote: add input feedback support for note errors

26e35d5

jb - EditableNote: add disabled state while note is being saved

f39c175

jb - Break out Note header into a separate component to be shared wit…

408e41a

…h other note classes

jb - EditableNote: support children for ease-of-reuse for custom notes

bd104a6

jb - Update documnetation for Notes and add for EditableNote

4e81572

joshuasbates requested review from gthomas-appfolio and mjewell June 15, 2018 20:50

gthomas-appfolio added 4 commits June 29, 2018 16:15

gt - Remove Notes header and controls

164f906

gt - Update tests

24f7955

gt - Add props per feedback

42fa96f

gt - Update stories

bd26e7d

joshuasbates commented Jul 2, 2018

View reviewed changes

sveinn reviewed Jul 2, 2018

View reviewed changes

joshuasbates commented Jul 2, 2018

View reviewed changes

sveinn reviewed Jul 2, 2018

View reviewed changes

joshuasbates force-pushed the update-notes-components branch from 30b132e to c680435 Compare July 2, 2018 21:29

jb - Minor cleanup doc cleanup; minor visual tweaks

66e68c4

joshuasbates force-pushed the update-notes-components branch from c680435 to 66e68c4 Compare July 2, 2018 21:29

jb - DeletedNote: Use Button instead of <a> for undo link (consistenc…

c496e8c

…y, recommended practice)

joshuasbates assigned joshuasbates and gthomas-appfolio Jul 3, 2018

gthomas-appfolio approved these changes Jul 3, 2018

View reviewed changes

gthomas-appfolio merged commit 8b530c6 into master Jul 3, 2018

phwebi deleted the update-notes-components branch January 29, 2020 01:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update notes components #419

Update notes components #419

joshuasbates commented Jun 15, 2018 •

edited

Loading

joshuasbates left a comment

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018 •

edited

Loading

joshuasbates Jul 2, 2018 •

edited

Loading

joshuasbates Jul 2, 2018

joshuasbates Jul 2, 2018

sveinn Jul 2, 2018

joshuasbates Jul 2, 2018

sveinn Jul 6, 2018

joshuasbates Jul 2, 2018 •

edited

Loading

joshuasbates Jul 2, 2018

sveinn Jul 2, 2018

joshuasbates Jul 2, 2018

sveinn Jul 6, 2018

gthomas-appfolio left a comment

		@@ -78,27 +78,26 @@ storiesOf('Notes', module)
		<Notes
		addControl={<EditableNote

		@@ -43,20 +44,18 @@ storiesOf('Notes', module)
		<Notes
		addControl={<EditableNote

Update notes components #419

Update notes components #419

Conversation

joshuasbates commented Jun 15, 2018 • edited Loading

joshuasbates 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

joshuasbates Jul 2, 2018 • edited Loading

Choose a reason for hiding this comment

joshuasbates Jul 2, 2018 • edited Loading

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

joshuasbates Jul 2, 2018 • edited Loading

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

gthomas-appfolio left a comment

Choose a reason for hiding this comment

joshuasbates commented Jun 15, 2018 •

edited

Loading

joshuasbates Jul 2, 2018 •

edited

Loading

joshuasbates Jul 2, 2018 •

edited

Loading

joshuasbates Jul 2, 2018 •

edited

Loading