Commit graph

96 commits

Author SHA1 Message Date
Ben Gotow
431a1adbec fix(specs): Resolve merge issues in specs 2015-07-31 12:06:34 -07:00
Ben Gotow
4be05ff754 feat(tracking-pixels): New MessageStore extension cuts out tracking pixels you've *sent* so you don't trigger them
Summary:
- Remove thread_participants prop, we don't use them anywhere and the underscore-case is ugly.

- Move autolinker into extension, update autolinker to 0.18.1 for phone number support

- document message.coffee, add isFromMe()

- Add tracking pixel extension that removes pixels from mail you *send*. Maybe more features later.

Test Plan: Run 1 new test! (woo...)

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1787
2015-07-23 10:57:13 -07:00
Evan Morikawa
45e27d7ded feat(labels): add a new label/folder picker
Summary:
This is the initial diff for the label picker UI. This is all of the
functionality and none of the CSS.

Test Plan: todo

Reviewers: bengotow

Reviewed By: bengotow

Subscribers: sdw

Differential Revision: https://phab.nylas.com/D1761
2015-07-21 14:20:15 -07:00
Evan Morikawa
00c74cd1a9 WIP: This is the initial diff for new folders & labels.
Summary:
There are now two objects, Folders & Labels. These inherit from `Category`
(that's what Eben said they were using on the backend).

There are two separate tasks.

1. MoveToFolderTask
2. ApplyLabelsTask

It turns out that the semantics between the two are quite different.
The reverse operation for moving to a folder is a bit tricky.

As of 7-8-15, the Tasks are pretty much complete. I need to write tests
for them still and do some manual testing in the client.

Test Plan: Writing specs

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1724
2015-07-16 11:54:20 -04:00
Ben Gotow
cc9d32b62f fix(reply-behavior): Reply keyboard shortcuts update existing draft rather than creating new ones
Summary:
When bcc or cc is added, the ComposerView should show the fields. We should never hide a populated field!

Move the logic for determining message reply / replyTo participant sets into the message object. This seems OK because the functions don't modify the message and deal entirely with message attributes

Fix miscelaneous scrollbar issue

The MessageList does not always fire a composeReply action in response to keyboard shortcuts and can update the existing draft instead

There are a couple goals here:
- If you have an existing draft, command-R and command-shift-R should never create a new draft, they should just update and focus the existing draft.
- If you actually press the Reply button on the message item it should still create a new draft. It's still possible to have two.
- If you press Command-R and add a participant, and then press Command-Shift-R, it should add participants but not blow away the one you've added manually.
- If you press Command-Shift-R and then Command-R, it should remove the participants that were part of the "reply all", but leave other participants you've added untouched.

Test Plan: Run new tests!

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T2140

Differential Revision: https://phab.nylas.com/D1734
2015-07-13 16:30:02 -07:00
Evan Morikawa
bd72a29409 feat(quoted-text): New quoted text engine
Summary:
Add email reply parser

Quoted text detection for HTML

More blockquote specs

Added 'On … wrote:'

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1719
2015-07-08 09:51:33 -07:00
Ben Gotow
343bb3163b feat(offline-mode, undo-redo): Tasks handle network errors better and retry, undo/redo based on tasks
Summary:
This diff does a couple things:

- Undo redo with a new undo/redo store that maintains it's own queue of undo/redo tasks. This queue is separate from the TaskQueue because not all tasks should be considered for undo history! Right now just the AddRemoveTagsTask is undoable.

- NylasAPI.makeRequest now returns a promise which resolves with the result or rejects with an error. For things that still need them, there's still `success` and `error` callbacks. I also added `started:(req) ->` which allows you to get the underlying request.

- Aborting a NylasAPI request now makes it call it's error callback / promise reject.

- You can now run code after perform local has completed using this syntax:

```
    task = new AddRemoveTagsTask(focused, ['archive'], ['inbox'])
    task.waitForPerformLocal().then ->
      Actions.setFocus(collection: 'thread', item: nextFocus)
      Actions.setCursorPosition(collection: 'thread', item: nextKeyboard)
    Actions.queueTask(task)
```

- In specs, you can now use `advanceClock` to get through a Promise.then/catch/finally. Turns out it was using something low level and not using setTimeout(0).

- The TaskQueue uses promises better and defers a lot of the complexity around queueState for performLocal/performRemote to a task subclass called APITask. APITask implements "perform" and breaks it into "performLocal" and "performRemote".

- All tasks either resolve or reject. They're always removed from the queue, unless they resolve with Task.Status.Retry, which means they internally did a .catch (err) => Promise.resolve(Task.Status.Retry) and they want to be run again later.

- API tasks retry until they succeed or receive a NylasAPI.PermanentErrorCode (400,404,500), in which case they revert and finish.

- The AddRemoveTags Task can now take more than one thread! This is super cool because you can undo/redo a bulk action and also because we'll probably have a bulk tag modification API endpoint soon.

Getting undo / redo working revealed that the thread versioning system we built isn't working because the server was incrementing things by more than 1 at a time. Now we count the number of unresolved "optimistic" changes we've made to a given model, and only accept the server's version of it once the number of optimistic changes is back at zero.

Known Issues:

- AddRemoveTagsTasks aren't dependent on each other, so if you (undo/redo x lots) and then come back online, all the tasks try to add / remove all the tags at the same time. To fix this we can either allow the tasks to be merged together into a minimal set or make them block on each other.

- When Offline, you still get errors in the console for GET requests. Need to catch these and display an offline status bar.

- The metadata tasks haven't been updated yet to the new API. Wanted to get it reviewed first!

Test Plan: All the tests still pass!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1694
2015-07-07 13:38:53 -04:00
Ben Gotow
4b8dd87410 fix(T2049): in 2-pane mode, keyboard actions are applied to selection, not message you are viewing
Fixes T2049 and moves ownership of the reply/reply-all/forward actions to the MessageList, which Rob did for starring and actually makes more sense.
2015-06-26 14:47:14 -07:00
Robert McQueen
84295b05d0 Add (s) hotkey for starring in both thread-list and message-list views
Summary:
Adding spec for message list star. Fixing multi-select in 3-pane starring

I didn't write a spec for thread-list because the thread-list-spec appears to be broken (returns immedietly at the top). I spent ~15 minutes attempting to fix but didn't make much progress

Test Plan: Added spec to message list

Reviewers: bengotow

Reviewed By: bengotow

Maniphest Tasks: T1957

Differential Revision: https://phab.nylas.com/D1697
2015-06-26 14:06:31 -07:00
Evan Morikawa
70a83a648d fix(*): minor fixes
Summary:
Fixes T1905 - Gost line in recipients field
Fixes T1877 - Collapsed dates should not show time

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Maniphest Tasks: T1877, T1905

Differential Revision: https://phab.nylas.com/D1671
2015-06-26 07:06:57 -07:00
Robert McQueen
28692ff5af feat(starring): Star/unstar from the message detail view
Summary: Adding star button to message-toolbar view

Test Plan: Added spec for starring message from the message-toolbar view

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1683
2015-06-25 10:33:26 -07:00
Robert McQueen
b42d102740 Include unnamed files as attachements and rename to 'Unnamed Attachement'
Summary: Fixes T2024 and renames unnamed files to Unnamed Attachment before appending the attachement component to message list

Test Plan: Added a test to ensure file without filename is renamed to Unnamed Attachment

Reviewers: bengotow

Reviewed By: bengotow

Maniphest Tasks: T2024

Differential Revision: https://phab.nylas.com/D1685
2015-06-25 09:51:46 -07:00
Evan Morikawa
4671bf885f feat(message): New Message List UI
Summary:
Initial message list collapsing

messages can be expanded explicitly

styling message items

composer UI and collapsing

expanding and collapsing headers

style new reply area

adding in message controls

Add message actions dropdown

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1664
2015-06-22 15:49:52 -07:00
Evan Morikawa
a48fc2dd34 feat(attachment): improved downloading and draggable images
Summary:
Fixes T1975
Fixes T1900
Fixes T1899
Fixes T1979

Attachments downloading update progress

downloads will restart if the file on disk isn't complete

can drag images onto drive

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Maniphest Tasks: T1900, T1899, T1975, T1979

Differential Revision: https://phab.nylas.com/D1638
2015-06-15 18:48:17 -07:00
Evan Morikawa
449e11cdda feat(attachments): new attachments & uploads UI with img support
Summary:
Initial styles on attachments ui

More file uploading UI

fix race condition in draft saving

attachments and uploads interweaved

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1610
2015-06-11 12:04:52 -07:00
Ben Gotow
4f41845a22 fix(draft-speed): Optimize draft creation and reduce scroll / focus delays
Summary:
This diff attempts to improve the responsiveness of the app when you hit "Reply". This is achieved by being smarter about creating the draft and loading it into the draft store, and also by allowing the compose* actions to take objects instead of just IDs (resulting in a fetch of the object).

Allow Actions.composeReply,etc. to optionally be called with thread and message objects instead of IDs. This prevents a database lookup and the data is "right there."

Create DraftStoreProxy for new drafts optimistically—this allows us to hand it the draft model we just created and it doesn't have to go query for it

When we create a new Draft, immediately bind it to a LocalId. This means that when the MessageStore receives the trigger() event from the Database, it doesn't have to wait while a localId is created

When MessageStore sees a new Message come in which is on the current thread, a draft, and not in the localIds map, assume it's a new draft and shortcut fetchFromCaceh to manually add it to the items array and display. This means the user sees the...

...draft instantly.

Remove delays from focusing draft, scrolling to draft after content is ready. I actually removed these thinking it would break something, and it didn't break anything.... Maybe new Chromium handles better?

Fix specs

Test Plan: Run specs - more in progress right now

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1598
2015-06-05 11:38:30 -07:00
Ben Gotow
a26b8d4bc4 fix(*): Resolve a variety of small and simple bugs
Summary:
Fix T1822 - saving templates not working, not showing template

Fix T1800 - give composers a minimum size

Fix the bottom bar of the composer so the gray bar goes all the way across in popout mode.

Fix T1825 - switch to a more attractive "June 4, 2015 at 3:10 PM" styling for expanded dates

Remove, rather than hide, react components for text fields in composer. Fixes T1147

Fix specs

Switch to 999+ instead of infinity. Fixes T1768

Fix broken TemplateStore specs

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T1147, T1768, T1822, T1800, T1825

Differential Revision: https://phab.nylas.com/D1601
2015-06-05 11:02:44 -07:00
Ben Gotow
9b37bfe4cb fix(T1744): Display attachments when message has no body, additional specs
Summary:
This fixes T1744 and adds specs for the logic in _attachmentComponents.
 - Attachments should display even when the body is an empty string or null
 - Attachments should not include files without names (mime parts of the message)

Test Plan: Run 2 new tests

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T1744

Differential Revision: https://phab.nylas.com/D1566
2015-05-28 18:15:47 -07:00
Evan Morikawa
fc4b3b56d7 refactor(utils): switch to regular underscore
Summary:
Fixes: T1334

remove final InboxApp references

move out all underscore-plus methods

Mass find and replace of underscore-plus

sed -i '' -- 's/underscore-plus/underscore/g' **/*.coffee
sed -i '' -- 's/underscore-plus/underscore/g' **/*.cjsx

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1534
2015-05-19 16:06:59 -07:00
Ben Gotow
9378f4480c fix(naming): Move atom/inbox/nilas refs to Nylas
Conflicts:
	internal_packages/inbox-activity-bar/lib/activity-bar-long-poll-item.cjsx
2015-05-15 11:07:28 -07:00
Ben Gotow
5b7ae0ad54 feat(tags): Tags dropdown and new features for Menu and Popover
Summary:
Focused Content Store should notify observers when focused items change, not just when they're reassigned

Popovers should have a `direction` and optional event onOpened

Menu divider documentation was wrong, menus should support checked items by default

Pressing escape in a popover's input should dismiss the popover

Other changes

Remove specs that make no sense anymore

Small tweak to report build breaking. Shouldn't happen often ;)

Test Plan: Run tests, which will now phone home if they break

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1493
2015-05-13 12:01:41 -07:00
Ben Gotow
9ffe0d74dd feat(unsafe-components): Wrap injected components, catch exceptions, clean up ComponentRegistry
Summary:
This diff gives the ComponentRegistry a cleaner, smaller API. Instead of querying by name, location or role,
it's now just location and role, and you can register components for one or more location and one or more
roles without assigning the entries in the registry separate names.

When you register with the ComponentRegistry, the syntax is also cleaner and uses the component's displayName
instead of requiring you to provide a name. You also provide the actual component when unregistering, ensuring
that you can't unregister someone else's component.

InjectedComponent and InjectedComponentSet now wrap their children in UnsafeComponent, which prevents
render/component lifecycle problems from propogating.

Existing components have been updated:

1. maxWidth / minWidth are now containerStyles.maxWidth/minWidth
2. displayName is now required to use the CR.
3. containerRequired = false can be provided to exempt a component from being wrapped in an UnsafeComponent.
   This is useful because it's slightly faster and keeps DOM flat.

This diff also makes the "Show Component Regions" more awesome. It displays column regions, since they now
use the InjectedComponentSet, and also shows for InjectedComponent as well as InjectedComponentSet.

Change ComponentRegistry syntax, lots more work on safely wrapping items. See description.

Fix for inline flexbox scenarios (message actions)

Allow ~/.inbox/packages to be symlinked to a github repo

Test Plan: Run tests!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1457
2015-04-30 16:10:15 -07:00
Ben Gotow
68343ec472 feat(docs): New docs tasks and React 0.13.2
Summary:
This diff moves us up to React 0.13.2 and transitions some of the core React components to the new
syntax based on plain Javascript objects. `setInitialState` is now just code in the constructor,
`getDOMNode(@)` is now `React.findDOMNode(@)`, and `isMounted` is no longer necessary or available.

This diff also adds `RegisteredComponent` to match `RegisteredRegion`. In another diff,
I think we should change the names of these to be `DynamicComponent` and `DynamicComponentSet`.

This diff also includes preliminary API Reference docs for Menu.cjsx and Popover.cjsx. You can build the docs
using `grunt docs` from the build folder. It produces a simple html format now, but it's easy
to customize.

Also we now ignore "Unnecessary fat arrow"

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1437
2015-04-24 11:33:10 -07:00
Ben Gotow
d9ee12cf81 refactor(*): Thread list fixes, flexible workspace store, multiple root sheets
Summary:
Remember to remove all the event listeners added to email frame

New files tab, queryable filename, not attribute

Rename ThreadSelectionBar to RootSelectionBar to go with RootCenterComponent, make it appear for draft selection and file selection as well

Initial file list and file list store, File Location

Remove unnecessary shouldComponentUpdate

Always track whether new requests have happened since ours to prevent out of order triggers

Always scroll to the current [focused/keyboard-cursor] in lists

So goodbye to the trash tag

Only scroll to current item if focus or keyboard has moved

Show message snippet in notification if no subject line

Make the RootSelectionBar pull items from Component Registry

New Archive button (prettier than the other one)

Refactor event additions to iframe so iframe can be used for file display also

Thread List is no longer the uber root package - drafts and files moved to separate packages

WorkspaceStore now allows packages to register sheets, "view" concept replaced with "root sheet" concept, "mode" may not be observed by all sheets, and is now called "preferred mode"

Don't animate transitions between two root sheets

Mode switch is only visible on root sheets that support multiple modes

Account sidebar now shows "Views" that have registered themselves: drafts and files for now

Model Selection Bar is now a component, just like ModelList. Meant to be in the toolbar above a Model List

Misc supporting changes

New files package which registers it's views and components

Rename files package to `file-list`

Move checkmark column down into model list

Don't throw exception if shift-down arrow and nothing selected

Takes a long time on login to fetch first page of threads, make pages smaller

Displaynames, spec fixes

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1412
2015-04-10 14:33:05 -07:00
Evan Morikawa
911d7f961d fix:(specs) for message list 2015-03-31 17:04:10 -07:00
Evan Morikawa
dc5941f3f1 feat(archive): archive now pops back to thread list
Summary:
add spec for message toolbar items

add thread list spec

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1344
2015-03-25 14:17:57 -04:00
Evan Morikawa
e1ec298d4b feat(messages): floating reply area in message-list
Summary:
reply text now scrolls to bottom on new draft

tests for reply type

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1338
2015-03-24 16:57:24 -04:00
Ben Gotow
e1edb2a220 fix(speed): Remove explicit .cjsx extensions from requires 2015-03-20 17:53:11 -07:00
Ben Gotow
29a7531fd1 fix(quoted-text): No quoted text style not being applied 2015-03-20 16:16:09 -07:00
Ben Gotow
b61863d332 fix(inline-attachments): Precompute sizes for better inline attachment loading
Summary: test case and fixes

Test Plan: Run new test!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1309
2015-03-18 13:54:34 -07:00
Ben Gotow
dee14a37b7 fix(message-list): Adds fade-in, fixes jerky scroll offset when viewing messages
Summary:
feat(debugging): Put db query tiemstamps in js timelines

Mark as read *after* you stop looking at a message, to avoid pointless repaint while looking

fix specs :'(

Make focus() do something smart if not provided a field name

Stop doing the partial load / full load from cache. Slower, but also means we can evaluate "correct" scroll offset in one pass

refactor message-list to fade in after load. simplifies scroll logic

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1306
2015-03-17 12:11:34 -07:00
Ben Gotow
fa69b07eaa fix(attachments): T427 - show attachments with contentIds not in body
Summary: Resolves an issue where an attachment with a contentId was excluded from the attachments list on the message, even though the cid was not found in the message body.

Test Plan: New test case

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T427

Differential Revision: https://review.inboxapp.com/D1293
2015-03-13 10:55:28 -07:00
Evan Morikawa
501704359e fix(participants): bcc field displays properly
Can also select text by clicking on the participant
2015-03-10 15:12:13 -07:00
Evan Morikawa
d5eb68ece1 fix(scrolling): scroll to end once. Fix draft deletion 2015-03-10 13:17:47 -07:00
Ben Gotow
df2c64f4dd fix(specs): Thread needs to be defined for MessageItem 2015-03-10 11:05:30 -07:00
Evan Morikawa
282cc40e9a feat(message): change long format timestamp
Summary: Minor UI fixes

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1271
2015-03-10 10:08:04 -07:00
Evan Morikawa
b479e099c1 feat(messages): expandable message headers
Summary: feat(messages): expandable message headers

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1267
2015-03-09 11:17:22 -07:00
Evan Morikawa
4635bf7d14 test fixes 2015-03-05 13:56:55 -08:00
Evan Morikawa
e7868df1ad feat(contenteditable): a React compatible contenteditable
Summary:
toolbar popup displays

restore caret protection on contenteditable

BAD - can't use cursor saving and restoring with react :(

_findNode works

saves and restores cursor state

contenteditable fixes to support cursor

comments on cursor

initial undo manager

extract undo manager and move up in stack

make undo manager a mixin

adding selection snapshots in composer

fixes in undo manager

selection saves selection states properly

move UndoManager and fix draft

selection state can now select backwards

selection works backwards and click not overridden

change bold class to allow for bolding and unbolding

styling of hover component

can set links in composer

bold and italic clicking works. text seleciton works

show link modal on hover

selection fixes

Test Plan: TODO

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1249
2015-03-02 15:33:58 -08:00
Ben Gotow
2a93c70214 fix(quoted-text): Show quoted text for short forwarded msgs
Summary: tiny patch, but with lots of tests for message-item and factored out message-timestamp

Test Plan: Run brand new specs!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1225
2015-02-24 16:20:57 -08:00
Ben Gotow
651f105740 fix(tests): Clean up after ReactTestUtils, wipe ComponentRegistry between specs
Summary:
Why does message-list have default participants? No other packages do

Component registry warns if mixin component name not found

Clear the component registry between tests and wipe React elements inserted into DOM

Everything should have a displayName, even you ComposerView

Stub all ComponentRegistry dependencies, always

Test Plan: Run all the tests at the same time

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1201
2015-02-18 14:09:46 -08:00
Evan Morikawa
e8533d8903 fix(composer): focus on creation and sanitize paste
Summary:
focuses on draft when replying

focus on to field on new composer

santize html on paste in contentedtiable

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1177
2015-02-16 17:09:28 -08:00
Evan Morikawa
a31c2808a9 fix(composer): make uploading attachments work
Summary:
There were several issues causing uploading to not work:

  # The file upload response came back as a string instead of an Object. Needed to detect and `JSON.parse`
  # The `MessageAttachment` component was not available as a package on the popout composer (since it used to be inside of `MessageList`)
  # The `message.draft` bit was not set properly causing the DB changes to be ignored
  # The actions notifying of `uploadStateChanged` were only being broadcasted in the main window (where the `TaskStore` is) and never making it to the popout composer.

Also keybindings for close window and up & down arrows were fixed.

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1157
2015-02-06 17:41:59 -05:00
Ben Gotow
db669fd9a9 feature(attachments): Inline attachments, download store does not use tasks
Summary:
This diff updates the FileDownloadStore to use it's own internal Download class instead of leveraging the TaskStore (since downloads shouldn't be resumable / serialized to disk anyway). This means things can be tighter and less dependent on Actions. Downloads are now promise-based, so actions like "open file after downloading" can be chained to the end of the Download promise.

The MessageStore now queues downloads for inline attachments when you view the thread, and the MessageItem.cjsx substitutes in local file paths for cid:<CID>.

Test Plan: I made some pretty big changes and did not break any tests. This is a bad sign... Tests forthcoming, but will probably ship this to users tomorrow first.

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1149
2015-02-06 13:13:31 -08:00
Evan Morikawa
4f8366a772 refactor(keymaps): override-key-bindings instead of native-key-bindings
Summary: This diff essentially inverts the behavior of native-key-bindings. Instead of opting-in to native-key-bindings, they're applied UNLESS there's an override-key-bindings class. I think this may be a better solution for us since we don't often want to override behavior like Copy and Select All.

Test Plan: No new tests on this one...

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://review.inboxapp.com/D1124
2015-02-04 21:31:41 -05:00
Ben Gotow
1e8fd46342 fix(drafts): Various improvements and fixes to drafts, draft state management
Summary:
This diff contains a few major changes:

1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario:
   - View thread with draft, edit draft
   - Move to another thread
   - Move back to thread with draft
   - Move to another thread. Notice that one or more messages from thread with draft are still there.

There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great.

2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here:

   - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI.

   - Previously, when you added a contact to To/CC/BCC, this happened:

     <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates

Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state.

To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source!

This diff includes a few minor fixes as well:

1. Draft.state is gone—use Message.object = draft instead
2. String model attributes should never be null
3. Pre-send checks that can cancel draft send
4. Put the entire curl history and task queue into feedback reports
5. Cache localIds for extra speed
6. Move us up to latest React

Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1125
2015-02-03 16:24:31 -08:00