mirror of
https://github.com/Foundry376/Mailspring.git
synced 2025-09-06 04:35:30 +08:00
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
This commit is contained in:
parent
5cc775fe3e
commit
a26b8d4bc4
12 changed files with 123 additions and 71 deletions
|
@ -159,44 +159,7 @@ class ComposerView extends React.Component
|
||||||
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<ParticipantsTextField
|
{@_renderFields()}
|
||||||
ref="textFieldTo"
|
|
||||||
field='to'
|
|
||||||
visible={true}
|
|
||||||
change={@_onChangeParticipants}
|
|
||||||
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
|
||||||
tabIndex='102'/>
|
|
||||||
|
|
||||||
<ParticipantsTextField
|
|
||||||
ref="textFieldCc"
|
|
||||||
field='cc'
|
|
||||||
visible={@state.showcc}
|
|
||||||
change={@_onChangeParticipants}
|
|
||||||
onEmptied={=> @setState showcc: false}
|
|
||||||
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
|
||||||
tabIndex='103'/>
|
|
||||||
|
|
||||||
<ParticipantsTextField
|
|
||||||
ref="textFieldBcc"
|
|
||||||
field='bcc'
|
|
||||||
visible={@state.showbcc}
|
|
||||||
change={@_onChangeParticipants}
|
|
||||||
onEmptied={=> @setState showbcc: false}
|
|
||||||
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
|
||||||
tabIndex='104'/>
|
|
||||||
|
|
||||||
<div className="compose-subject-wrap"
|
|
||||||
style={display: @state.showsubject and 'initial' or 'none'}>
|
|
||||||
<input type="text"
|
|
||||||
key="subject"
|
|
||||||
name="subject"
|
|
||||||
tabIndex="108"
|
|
||||||
placeholder="Subject:"
|
|
||||||
disabled={not @state.showsubject}
|
|
||||||
className="compose-field compose-subject"
|
|
||||||
value={@state.subject}
|
|
||||||
onChange={@_onChangeSubject}/>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
<div className="compose-body">
|
<div className="compose-body">
|
||||||
<ContenteditableComponent ref="contentBody"
|
<ContenteditableComponent ref="contentBody"
|
||||||
|
@ -218,6 +181,58 @@ class ComposerView extends React.Component
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
_renderFields: =>
|
||||||
|
# Note: We need to physically add and remove these elements, not just hide them.
|
||||||
|
# If they're hidden, shift-tab between fields breaks.
|
||||||
|
fields = []
|
||||||
|
fields.push(
|
||||||
|
<ParticipantsTextField
|
||||||
|
ref="textFieldTo"
|
||||||
|
field='to'
|
||||||
|
change={@_onChangeParticipants}
|
||||||
|
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
||||||
|
tabIndex='102'/>
|
||||||
|
)
|
||||||
|
|
||||||
|
if @state.showcc
|
||||||
|
fields.push(
|
||||||
|
<ParticipantsTextField
|
||||||
|
ref="textFieldCc"
|
||||||
|
field='cc'
|
||||||
|
change={@_onChangeParticipants}
|
||||||
|
onEmptied={=> @setState showcc: false}
|
||||||
|
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
||||||
|
tabIndex='103'/>
|
||||||
|
)
|
||||||
|
|
||||||
|
if @state.showbcc
|
||||||
|
fields.push(
|
||||||
|
<ParticipantsTextField
|
||||||
|
ref="textFieldBcc"
|
||||||
|
field='bcc'
|
||||||
|
change={@_onChangeParticipants}
|
||||||
|
onEmptied={=> @setState showbcc: false}
|
||||||
|
participants={to: @state['to'], cc: @state['cc'], bcc: @state['bcc']}
|
||||||
|
tabIndex='104'/>
|
||||||
|
)
|
||||||
|
|
||||||
|
if @state.showsubject
|
||||||
|
fields.push(
|
||||||
|
<div className="compose-subject-wrap">
|
||||||
|
<input type="text"
|
||||||
|
key="subject"
|
||||||
|
name="subject"
|
||||||
|
tabIndex="108"
|
||||||
|
placeholder="Subject:"
|
||||||
|
disabled={not @state.showsubject}
|
||||||
|
className="compose-field compose-subject"
|
||||||
|
value={@state.subject}
|
||||||
|
onChange={@_onChangeSubject}/>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
|
||||||
|
fields
|
||||||
|
|
||||||
_renderFooterRegions: =>
|
_renderFooterRegions: =>
|
||||||
return <div></div> unless @props.localId
|
return <div></div> unless @props.localId
|
||||||
|
|
||||||
|
|
|
@ -43,12 +43,13 @@ class ContenteditableComponent extends React.Component
|
||||||
range = @_getRangeInScope()
|
range = @_getRangeInScope()
|
||||||
for extension in DraftStore.extensions()
|
for extension in DraftStore.extensions()
|
||||||
extension.onFocusNext(editableNode, range, event) if extension.onFocusNext
|
extension.onFocusNext(editableNode, range, event) if extension.onFocusNext
|
||||||
|
|
||||||
'core:focus-previous': (event) =>
|
'core:focus-previous': (event) =>
|
||||||
editableNode = @_editableNode()
|
editableNode = @_editableNode()
|
||||||
range = @_getRangeInScope()
|
range = @_getRangeInScope()
|
||||||
for extension in DraftStore.extensions()
|
for extension in DraftStore.extensions()
|
||||||
extension.onFocusPrevious(editableNode, range, event) if extension.onFocusPrevious
|
extension.onFocusPrevious(editableNode, range, event) if extension.onFocusPrevious
|
||||||
}
|
}
|
||||||
|
|
||||||
@_cleanHTML()
|
@_cleanHTML()
|
||||||
|
|
||||||
|
|
|
@ -55,6 +55,7 @@ module.exports =
|
||||||
ComponentRegistry.register ComposeButton,
|
ComponentRegistry.register ComposeButton,
|
||||||
location: WorkspaceStore.Location.RootSidebar.Toolbar
|
location: WorkspaceStore.Location.RootSidebar.Toolbar
|
||||||
else
|
else
|
||||||
|
atom.getCurrentWindow().setMinimumSize(600, 400)
|
||||||
WorkspaceStore.defineSheet 'Main', {root: true},
|
WorkspaceStore.defineSheet 'Main', {root: true},
|
||||||
list: ['Center']
|
list: ['Center']
|
||||||
ComponentRegistry.register ComposerWithWindowProps,
|
ComponentRegistry.register ComposerWithWindowProps,
|
||||||
|
|
|
@ -17,9 +17,6 @@ class ParticipantsTextField extends React.Component
|
||||||
# to modify the `participants` provided.
|
# to modify the `participants` provided.
|
||||||
field: React.PropTypes.string,
|
field: React.PropTypes.string,
|
||||||
|
|
||||||
# Whether or not the field should be visible. Defaults to true.
|
|
||||||
visible: React.PropTypes.bool
|
|
||||||
|
|
||||||
# An object containing arrays of participants. Typically, this is
|
# An object containing arrays of participants. Typically, this is
|
||||||
# {to: [], cc: [], bcc: []}. Each ParticipantsTextField needs all of
|
# {to: [], cc: [], bcc: []}. Each ParticipantsTextField needs all of
|
||||||
# the values, because adding an element to one field may remove it
|
# the values, because adding an element to one field may remove it
|
||||||
|
@ -36,7 +33,7 @@ class ParticipantsTextField extends React.Component
|
||||||
render: =>
|
render: =>
|
||||||
classSet = {}
|
classSet = {}
|
||||||
classSet[@props.field] = true
|
classSet[@props.field] = true
|
||||||
<div className="participants-text-field" style={zIndex: 1000-@props.tabIndex, display: @props.visible and 'inline' or 'none'}>
|
<div className="participants-text-field" style={zIndex: 1000-@props.tabIndex, display: 'inline'}>
|
||||||
<TokenizingTextField
|
<TokenizingTextField
|
||||||
ref="textField"
|
ref="textField"
|
||||||
tokens={@props.participants[@props.field]}
|
tokens={@props.participants[@props.field]}
|
||||||
|
|
|
@ -182,7 +182,7 @@ describe "populated composer", ->
|
||||||
describe "when focus() is called", ->
|
describe "when focus() is called", ->
|
||||||
describe "if a field name is provided", ->
|
describe "if a field name is provided", ->
|
||||||
it "should focus that field", ->
|
it "should focus that field", ->
|
||||||
useDraft.call(@)
|
useDraft.call(@, cc: [u2])
|
||||||
makeComposer.call(@)
|
makeComposer.call(@)
|
||||||
spyOn(@composer.refs['textFieldCc'], 'focus')
|
spyOn(@composer.refs['textFieldCc'], 'focus')
|
||||||
@composer.focus('textFieldCc')
|
@composer.focus('textFieldCc')
|
||||||
|
|
|
@ -28,15 +28,15 @@
|
||||||
width: 100%;
|
width: 100%;
|
||||||
background: transparent;
|
background: transparent;
|
||||||
border-bottom: 0;
|
border-bottom: 0;
|
||||||
max-width: @compose-width;
|
|
||||||
margin: 0 auto;
|
|
||||||
|
|
||||||
.composer-action-bar-content {
|
.composer-action-bar-content {
|
||||||
display:flex;
|
display:flex;
|
||||||
|
margin: 0 auto;
|
||||||
flex-direction:row;
|
flex-direction:row;
|
||||||
margin-left: -@spacing-standard / 2;
|
max-width: @compose-width;
|
||||||
margin-right: -@spacing-standard / 2;
|
padding-left: @spacing-standard + @spacing-standard / 2;
|
||||||
padding: @spacing-standard @spacing-double;
|
padding-right: @spacing-standard + @spacing-standard / 2;
|
||||||
|
padding-top: @spacing-standard;
|
||||||
padding-bottom: @spacing-standard*1.1;
|
padding-bottom: @spacing-standard*1.1;
|
||||||
|
|
||||||
> * {
|
> * {
|
||||||
|
@ -109,7 +109,7 @@
|
||||||
margin: 0 @spacing-standard;
|
margin: 0 @spacing-standard;
|
||||||
border-bottom: 1px solid @border-color-divider;
|
border-bottom: 1px solid @border-color-divider;
|
||||||
flex-shrink:0;
|
flex-shrink:0;
|
||||||
|
|
||||||
.subject-label {
|
.subject-label {
|
||||||
color: @text-color-very-subtle;
|
color: @text-color-very-subtle;
|
||||||
float: left;
|
float: left;
|
||||||
|
|
|
@ -23,7 +23,7 @@ class MessageTimestamp extends React.Component
|
||||||
|
|
||||||
_timeFormat: =>
|
_timeFormat: =>
|
||||||
if @props.isDetailed
|
if @props.isDetailed
|
||||||
return "DD / MM / YYYY h:mm a z"
|
return "MMMM D, YYYY [at] h:mm A"
|
||||||
else
|
else
|
||||||
today = moment(@_today())
|
today = moment(@_today())
|
||||||
dayOfEra = today.dayOfYear() + today.year() * 365
|
dayOfEra = today.dayOfYear() + today.year() * 365
|
||||||
|
|
|
@ -38,4 +38,4 @@ describe "MessageTimestamp", ->
|
||||||
<MessageTimestamp date={testDate()} isDetailed={true} />
|
<MessageTimestamp date={testDate()} isDetailed={true} />
|
||||||
)
|
)
|
||||||
spyOn(itemDetailed, "_today").andCallFake -> testDate()
|
spyOn(itemDetailed, "_today").andCallFake -> testDate()
|
||||||
expect(itemDetailed._timeFormat()).toBe "DD / MM / YYYY h:mm a z"
|
expect(itemDetailed._timeFormat()).toBe "MMMM D, YYYY [at] h:mm A"
|
||||||
|
|
|
@ -37,7 +37,7 @@ class TemplatePicker extends React.Component
|
||||||
]
|
]
|
||||||
|
|
||||||
footerComponents = [
|
footerComponents = [
|
||||||
<div className="item" key="new" onMouseDown={@_onNewTemplate}>Save as Template...</div>
|
<div className="item" key="new" onMouseDown={@_onNewTemplate}>Save Draft as Template...</div>
|
||||||
<div className="item" key="manage" onMouseDown={@_onManageTemplates}>Open Templates Folder...</div>
|
<div className="item" key="manage" onMouseDown={@_onManageTemplates}>Open Templates Folder...</div>
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
Reflux = require 'reflux'
|
Reflux = require 'reflux'
|
||||||
_ = require 'underscore'
|
_ = require 'underscore'
|
||||||
{DatabaseStore, DraftStore, Actions, Message} = require 'nylas-exports'
|
{DatabaseStore, DraftStore, Actions, Message} = require 'nylas-exports'
|
||||||
|
shell = require 'shell'
|
||||||
path = require 'path'
|
path = require 'path'
|
||||||
fs = require 'fs-plus'
|
fs = require 'fs-plus'
|
||||||
|
|
||||||
|
@ -59,21 +60,32 @@ TemplateStore = Reflux.createStore
|
||||||
draft = session.draft()
|
draft = session.draft()
|
||||||
name ?= draft.subject
|
name ?= draft.subject
|
||||||
contents ?= draft.body
|
contents ?= draft.body
|
||||||
|
if not name or name.length is 0
|
||||||
|
return @_displayError("Give your draft a subject to name your template.")
|
||||||
|
if not contents or contents.length is 0
|
||||||
|
return @_displayError("To create a template you need to fill the body of the current draft.")
|
||||||
@_writeTemplate(name, contents)
|
@_writeTemplate(name, contents)
|
||||||
|
|
||||||
else
|
else
|
||||||
|
if not name or name.length is 0
|
||||||
|
return @_displayError("You must provide a name for your template.")
|
||||||
|
if not contents or contents.length is 0
|
||||||
|
return @_displayError("You must provide contents for your template.")
|
||||||
@_writeTemplate(name, contents)
|
@_writeTemplate(name, contents)
|
||||||
|
|
||||||
_onShowTemplates: ->
|
_onShowTemplates: ->
|
||||||
# show in finder how?
|
|
||||||
shell = require 'shell'
|
|
||||||
shell.showItemInFolder(@_items[0]?.path || @_templatesDir)
|
shell.showItemInFolder(@_items[0]?.path || @_templatesDir)
|
||||||
|
|
||||||
|
_displayError: (message) ->
|
||||||
|
dialog = require('remote').require('dialog')
|
||||||
|
dialog.showErrorBox('Template Creation Error', message)
|
||||||
|
|
||||||
_writeTemplate: (name, contents) ->
|
_writeTemplate: (name, contents) ->
|
||||||
throw new Error("You must provide a template name") unless name
|
|
||||||
throw new Error("You must provide template contents") unless contents
|
|
||||||
filename = "#{name}.html"
|
filename = "#{name}.html"
|
||||||
templatePath = path.join(@_templatesDir, filename)
|
templatePath = path.join(@_templatesDir, filename)
|
||||||
fs.writeFile templatePath, contents, (err) =>
|
fs.writeFile templatePath, contents, (err) =>
|
||||||
|
@_displayError(err) if err
|
||||||
|
shell.showItemInFolder(templatePath)
|
||||||
@_items.push
|
@_items.push
|
||||||
id: filename,
|
id: filename,
|
||||||
name: name,
|
name: name,
|
||||||
|
|
|
@ -18,6 +18,9 @@ stubTemplates = [
|
||||||
describe "TemplateStore", ->
|
describe "TemplateStore", ->
|
||||||
beforeEach ->
|
beforeEach ->
|
||||||
spyOn(fs, 'mkdir')
|
spyOn(fs, 'mkdir')
|
||||||
|
spyOn(shell, 'showItemInFolder').andCallFake ->
|
||||||
|
spyOn(fs, 'writeFile').andCallFake (path, contents, callback) ->
|
||||||
|
callback(null)
|
||||||
spyOn(fs, 'readFile').andCallFake (path, callback) ->
|
spyOn(fs, 'readFile').andCallFake (path, callback) ->
|
||||||
filename = path.split('/').pop()
|
filename = path.split('/').pop()
|
||||||
callback(null, stubTemplateFiles[filename])
|
callback(null, stubTemplateFiles[filename])
|
||||||
|
@ -71,9 +74,15 @@ describe "TemplateStore", ->
|
||||||
|
|
||||||
describe "onCreateTemplate", ->
|
describe "onCreateTemplate", ->
|
||||||
beforeEach ->
|
beforeEach ->
|
||||||
spyOn(fs, 'readdir').andCallFake (path, callback) -> callback(null, [])
|
|
||||||
spyOn(fs, 'writeFile').andCallFake (path, contents, callback) -> callback(null)
|
|
||||||
TemplateStore.init()
|
TemplateStore.init()
|
||||||
|
spyOn(DraftStore, 'sessionForLocalId').andCallFake (draftLocalId) ->
|
||||||
|
if draftLocalId is 'localid-nosubject'
|
||||||
|
d = new Message(subject: '', body: '<p>Body</p>')
|
||||||
|
else
|
||||||
|
d = new Message(subject: 'Subject', body: '<p>Body</p>')
|
||||||
|
session =
|
||||||
|
draft: -> d
|
||||||
|
Promise.resolve(session)
|
||||||
|
|
||||||
it "should create a template with the given name and contents", ->
|
it "should create a template with the given name and contents", ->
|
||||||
TemplateStore._onCreateTemplate({name: '123', contents: 'bla'})
|
TemplateStore._onCreateTemplate({name: '123', contents: 'bla'})
|
||||||
|
@ -82,11 +91,15 @@ describe "TemplateStore", ->
|
||||||
expect(item.name).toBe "123"
|
expect(item.name).toBe "123"
|
||||||
expect(item.path.split("/").pop()).toBe "123.html"
|
expect(item.path.split("/").pop()).toBe "123.html"
|
||||||
|
|
||||||
it "should throw an exception if no name is provided", ->
|
it "should display an error if no name is provided", ->
|
||||||
expect( -> TemplateStore._onCreateTemplate({contents: 'bla'})).toThrow()
|
spyOn(TemplateStore, '_displayError')
|
||||||
|
TemplateStore._onCreateTemplate({contents: 'bla'})
|
||||||
|
expect(TemplateStore._displayError).toHaveBeenCalled()
|
||||||
|
|
||||||
it "should throw an exception if no content is provided", ->
|
it "should display an error if no content is provided", ->
|
||||||
expect( -> TemplateStore._onCreateTemplate({name: 'bla'})).toThrow()
|
spyOn(TemplateStore, '_displayError')
|
||||||
|
TemplateStore._onCreateTemplate({name: 'bla'})
|
||||||
|
expect(TemplateStore._displayError).toHaveBeenCalled()
|
||||||
|
|
||||||
it "should save the template file to the templates folder", ->
|
it "should save the template file to the templates folder", ->
|
||||||
TemplateStore._onCreateTemplate({name: '123', contents: 'bla'})
|
TemplateStore._onCreateTemplate({name: '123', contents: 'bla'})
|
||||||
|
@ -95,17 +108,30 @@ describe "TemplateStore", ->
|
||||||
expect(fs.writeFile.mostRecentCall.args[0]).toEqual(path)
|
expect(fs.writeFile.mostRecentCall.args[0]).toEqual(path)
|
||||||
expect(fs.writeFile.mostRecentCall.args[1]).toEqual('bla')
|
expect(fs.writeFile.mostRecentCall.args[1]).toEqual('bla')
|
||||||
|
|
||||||
|
it "should open the template so you can see it", ->
|
||||||
|
TemplateStore._onCreateTemplate({name: '123', contents: 'bla'})
|
||||||
|
path = "#{stubTemplatesDir}/123.html"
|
||||||
|
expect(shell.showItemInFolder).toHaveBeenCalled()
|
||||||
|
|
||||||
describe "when given a draft id", ->
|
describe "when given a draft id", ->
|
||||||
it "should create a template from the name and contents of the given draft", ->
|
it "should create a template from the name and contents of the given draft", ->
|
||||||
draft = new Message
|
runs ->
|
||||||
subject: 'Subject'
|
TemplateStore._onCreateTemplate({draftLocalId: 'localid-b'})
|
||||||
body: '<p>Body</p>'
|
waitsFor ->
|
||||||
spyOn(DatabaseStore, 'findByLocalId').andReturn(Promise.resolve(draft))
|
fs.writeFile.callCount > 0
|
||||||
TemplateStore._onCreateTemplate({draftLocalId: 'localid-b'})
|
runs ->
|
||||||
expect(TemplateStore.items()).toEqual([])
|
expect(TemplateStore.items().length).toEqual(1)
|
||||||
|
|
||||||
|
it "should display an error if the draft has no subject", ->
|
||||||
|
spyOn(TemplateStore, '_displayError')
|
||||||
|
runs ->
|
||||||
|
TemplateStore._onCreateTemplate({draftLocalId: 'localid-nosubject'})
|
||||||
|
waitsFor ->
|
||||||
|
TemplateStore._displayError.callCount > 0
|
||||||
|
runs ->
|
||||||
|
expect(TemplateStore._displayError).toHaveBeenCalled()
|
||||||
|
|
||||||
describe "onShowTemplates", ->
|
describe "onShowTemplates", ->
|
||||||
it "should open the templates folder in the Finder", ->
|
it "should open the templates folder in the Finder", ->
|
||||||
spyOn(shell, 'showItemInFolder')
|
|
||||||
TemplateStore._onShowTemplates()
|
TemplateStore._onShowTemplates()
|
||||||
expect(shell.showItemInFolder).toHaveBeenCalled()
|
expect(shell.showItemInFolder).toHaveBeenCalled()
|
||||||
|
|
|
@ -36,7 +36,7 @@ AppUnreadBadgeStore = Reflux.createStore
|
||||||
AppUnreadCount = count
|
AppUnreadCount = count
|
||||||
|
|
||||||
if count > 999
|
if count > 999
|
||||||
app.dock?.setBadge?("\u221E")
|
app.dock?.setBadge?("999+")
|
||||||
else if count > 0
|
else if count > 0
|
||||||
app.dock?.setBadge?("#{count}")
|
app.dock?.setBadge?("#{count}")
|
||||||
else
|
else
|
||||||
|
|
Loading…
Add table
Reference in a new issue