Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update notes components #419

Merged
merged 17 commits into from
Jul 3, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
567cfeb
Revert "jb - Update BoundForm to allow update of object through new p…
joshuasbates May 25, 2018
2921fea
Merge branch 'master' of https://github.com/appfolio/react-gears
joshuasbates Jun 8, 2018
eaf5781
Merge branch 'master' of https://github.com/appfolio/react-gears
joshuasbates Jun 15, 2018
cbdfeb5
jb - Make Notes help bubble text more customizable
joshuasbates May 25, 2018
15c8c25
jb - Render Notes as a block panel; add missing tests
joshuasbates May 25, 2018
1ee453d
jb - EditableNote: update callbacks to specify the note; propagate cl…
joshuasbates May 29, 2018
26e35d5
jb - EditableNote: add input feedback support for note errors
joshuasbates May 29, 2018
f39c175
jb - EditableNote: add disabled state while note is being saved
joshuasbates May 29, 2018
408e41a
jb - Break out Note header into a separate component to be shared wit…
joshuasbates May 30, 2018
bd104a6
jb - EditableNote: support children for ease-of-reuse for custom notes
joshuasbates May 30, 2018
4e81572
jb - Update documnetation for Notes and add for EditableNote
joshuasbates Jun 15, 2018
164f906
gt - Remove Notes header and controls
gthomas-appfolio Jun 29, 2018
24f7955
gt - Update tests
gthomas-appfolio Jun 29, 2018
42fa96f
gt - Add props per feedback
gthomas-appfolio Jun 29, 2018
bd26e7d
gt - Update stories
gthomas-appfolio Jun 29, 2018
66e68c4
jb - Minor cleanup doc cleanup; minor visual tweaks
joshuasbates Jul 2, 2018
c496e8c
jb - DeletedNote: Use Button instead of <a> for undo link (consistenc…
joshuasbates Jul 3, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/components/EditableNote.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,35 @@ class EditableNote extends React.Component {
note: PropTypes.shape({
date: PropTypes.object,
errors: PropTypes.string,
saving: PropTypes.bool,
text: PropTypes.string
}),
onCancel: PropTypes.func.isRequired,
onChange: PropTypes.func.isRequired,
onSave: PropTypes.func.isRequired
onSave: PropTypes.func.isRequired,
rows: PropTypes.number,
saving: PropTypes.bool
};

static defaultProps = {
className: 'bg-white mb-3'
className: 'bg-white mb-3',
rows: 4,
saving: false
};

render() {
const { children, className, note, onCancel, onChange, onSave } = this.props;
const { date, errors, saving, text } = note;
const { children, className, note, onCancel, onChange, onSave, rows, saving } = this.props;
const { date, errors, text } = note;

return (
<Card className={className}>
{date && <NoteHeader note={note} />}
<CardBlock>
<FormLabelGroup feedback={errors} stacked>
<Input
autoFocus
disabled={saving}
ref="text"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this ref for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, just curious

rows="4"
rows={rows}
state={errors && 'danger'}
type="textarea"
value={text}
Expand Down
12 changes: 9 additions & 3 deletions src/components/Note.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,19 @@ class Note extends React.Component {
onDelete: PropTypes.func,
onEdit: PropTypes.func,
onSave: PropTypes.func,
onUndelete: PropTypes.func
onUndelete: PropTypes.func,
rows: PropTypes.number,
saving: PropTypes.boolean
};

static defaultProps = {
className: 'bg-white mb-3'
className: 'bg-white mb-3',
rows: EditableNote.defaultProps.rows,
saving: EditableNote.defaultProps.saving
};

render() {
const { children, className, note, onCancel, onChange, onDelete, onEdit, onSave, onUndelete }
const { children, className, note, onCancel, onChange, onDelete, onEdit, onSave, onUndelete, rows, saving }
= this.props;
const { deleted, editing, text } = note;

Expand All @@ -51,6 +55,8 @@ class Note extends React.Component {
onCancel={onCancel}
onChange={onChange}
onSave={onSave}
rows={rows}
saving={saving}
/>);
}
return (
Expand Down
41 changes: 28 additions & 13 deletions src/components/NoteHeader.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fecha from 'fecha';
import PropTypes from 'prop-types';
import React from 'react';

import classnames from 'classnames';
import Badge from './Badge';
import Button from './Button';
import CardHeader from './CardHeader';
Expand All @@ -26,22 +26,37 @@ class NoteHeader extends React.Component {
const { note, onDelete, onEdit } = this.props;
const { date, edited, from } = note;

const headerClassNames = classnames(
'd-flex',
'flex-wrap',
'align-items-center',
'justify-content-between',
'py-2',
'pr-2',
'bg-info'
);

return (
<CardHeader className="d-flex justify-content-start p-2 bg-info">
{edited ? <span className="js-note-header__edited"><Badge color="primary text-uppercase mr-2">Edited</Badge></span> : null}
<span className="text-muted">
<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>
</span>
<span className="hidden-sm-up">
{from ? <span>{from} </span> : null}<span className="js-note-header__shortDate">{dateFormat(date, 'M/D/YY h:mm A')}</span>
<CardHeader className={headerClassNames}>
<div
className="d-inline-flex align-items-center"
ref="title"
>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

{edited && <Badge color="primary" className="text-uppercase mr-2 js-note-header__edited">Edited</Badge>}
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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"

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

</span>
<span className="hidden-sm-up">
{from ? <span>{from} </span> : null}<span className="js-note-header__shortDate">{dateFormat(date, 'M/D/YY h:mm A')}</span>
</span>
</span>
</span>
<span className="ml-auto">
</div>
<div className="d-inline-flex">
{onEdit ? <Button color="link" onClick={() => onEdit(note)} className="js-note-header__edit mr-3 p-0">edit</Button> : null}
{onDelete ? <Button color="link" onClick={() => onDelete(note)} className="js-note-header__delete p-0">delete</Button> : null}
</span>
</div>
</CardHeader>
);
}
Expand Down
3 changes: 1 addition & 2 deletions test/components/EditableNote.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ describe('<EditableNote />', () => {

context('in saving mode', () => {
beforeEach(() => {
props.note.saving = true;
component = mount(<EditableNote {...props} />);
component = mount(<EditableNote saving {...props} />);
});

it('should render text input disabled', () => {
Expand Down