Commit graph

18 commits

Author SHA1 Message Date
Ben Gotow ded4da1505 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
Evan Morikawa 4619871e8d 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 91edef9f7a 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 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
Evan Morikawa 911d7f961d fix:(specs) for message list 2015-03-31 17:04:10 -07: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 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
Evan Morikawa d5eb68ece1 fix(scrolling): scroll to end once. Fix draft deletion 2015-03-10 13:17:47 -07:00
Evan Morikawa 4635bf7d14 test fixes 2015-03-05 13:56:55 -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