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

Implements Outlook add-in commands #125

Merged
merged 29 commits into from
Jan 26, 2016
Merged

Implements Outlook add-in commands #125

merged 29 commits into from
Jan 26, 2016

Conversation

jasonjoh
Copy link
Contributor

I implemented this as a subgenerator called ‘commands’. I designed it with two scenarios in mind:

  • One of the existing subgenerators (‘mail’, ‘content’, etc) invokes it as part of their processing
  • The user calls ‘commands’ directly to update an existing manifest

For full details see #55 (comment)

Resolves #55

So far I've got this working when run on the output of running yo office
to build an HTML/JS Mail addin. To run this, you enter 'yo
office:commands' and go through the prompts.

Not working:
- CustomPane
- Custom tab
Turns out I was setting the same id for each custom tab, invalidating
the manifest.
- Removed method to prompt for CustomPane parameters to simplify the
flow. Now just creates a standard sample.
- Removed outlookForm as an option and a prompt, replaced with
extensionPoint, which applies to Outlook and others
- Added prompt for extensionPoint in mail generator, only gives options
that are valid for mail add-ins
Added option to pass predefined commands and functions for buttons

This is mainly to support customizing an existing sample, like the
'html' sample in the mail generator. The sample template can include an
XML stub with the Hosts and Resources nodes filled in for the manifest,
and a JS file with functions to add to the function file.
Fixing issues reported by gulp vet

Renamed Functions.js to Functions.ejs since it uses the EJS template
language, which confused the gulp vet task.
Updated existing tests to use new options for the mail generator.

Because the old 'outlookForm' option was removed and replaced with
'extensionPoint', the tests needed to be updated to use the equivalent
extension points.
Added tests for scenario where commands generator is invoked by the mail
generator
Added test for tech="html" and
extensionPoint="MessageReadCommandSurface,
MessageComposeCommandSurface"]

The generator should remove the unneeded entries from the manifest,
currently it does not.
Testing revealed a couple of issues, this checkin fixes them.

- When running yo office and selecting Mail, HTML/CSS, if you select
CustomPane a CustomPane was not added to the manifest
- When running yo office and selecting Mail, HTML/CSS, if you unselect
any of the pre-selected extension points, it was added to the manifest
anyway.

Added a CustomPane template to the command data for the HTML template in
the mail generator.
Added a function to remove unused entries from the command data before
writing it to the manifest.
Adding tests for scenario when commands generator is invoked on its own.

Also corrected a typo in invoked-by-mail.js
Fixing issue where sample files were created even if the generator
failed.

When invoking commands generator on its own with a non-existent
manifest, it was returning an error but still copying over sample files.
Updated to use new option
Numerous 'line too long' warnings removed.
Added a dropdown menu to compose forms in the HTML/CSS template.

Moved the "Set subject" and "Add recipient" buttons into the menu.
Added new tests and cleaned up code to remove unused bits.
Removed the 'Tooltip' element from buttons.

The element itself is optional in the schema, and Outlook never displays
it anyway. Removing it removes bloat from the manifest.
"Creates expected files" tests are failing on travis-ci.org but succeed
on my local. Guessing that it may be a case-sensitivity issue, so
changing the file paths to use the same case as what gets created.
My last checkin seemed to resolve some of the Travis issues, so making
similar changes to case in file paths everywhere.
Ok, I guess I went a little too far with the last change. This should
hopefully fix it.
- Added commands to list of subgenerators
- Added explanations of extension points
@msftclas
Copy link

Hi @jasonjoh, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Jason Johnston). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@jthake
Copy link
Contributor

jthake commented Jan 15, 2016

I just tested this. When you select HTML/JS option for Mail you do not get the add-in command questions like you do if you select Angular. is there a reason this hasn't been done? This change needs to be reflected on all the options for mail for consistency.

For manifest only - Is there a way where it prompts for function file, task pane url, icon url can have examples in brackets to make it more obvious what format it needs?

@jasonjoh
Copy link
Contributor Author

The HTML/JS sample doesn't prompt you because it supplies all of the values in an overrides.xml file. This is done so that the buttons that get created mirror the existing sample, creating similar functionality in a client that supports commands and one that doesn't.

The Angular sample doesn't have any sample functionality, so it doesn't use overrides.xml.

For manifest only, I can certainly add prompts for this. Is that something that needs to be done before we get it merged?

@jthake
Copy link
Contributor

jthake commented Jan 19, 2016

I'd personally rather have a complete picture than have it work for just one scenario as it'll confuse users. How much work is it for this? I know this means its a bit more work.

I think we need to drop the sample out of HTML so that this also can handle add-in commands. I will create an issue for that. #126 covers this.

@jasonjoh
Copy link
Contributor Author

Well to be clear, the HTML option does handle add-in commands, it just doesn't ask the user for it. The subgenerator still generates all the entries in the XML for you. This was done to minimize how much we have to ask the user for and give the calling generator more control over the end result.

It might make sense to have a quick chat about this so there's no confusion :)

@jthake
Copy link
Contributor

jthake commented Jan 20, 2016

Can we be consistent with the HTML questions and the Angular one and Manifest only one? Can we just assume that all ask the same questions?

@jasonjoh
Copy link
Contributor Author

Well, if we do #126, it would do that. The commands subgenerator doesn't choose to not prompt based on the tech, it chooses not to prompt based on the presence of a JSON object pre-defining all of the commands that gets passed by the mail generator. My concern about making it prompt for commands while the HTML sample is still there is that it creates a disjointed experience. OWA or Mac Outlook users would see the old UI with "Get Subject, Set Subject, etc", while Outlook 2016 users would see the generated buttons "UI-less Button 1", etc.

Either way I think it leaves the HTML/JS scenario in an odd place until #126 renders it moot. We either have a sample with wildly different scenarios depending on client, or we have the potentially confusing lack of prompts. I can do either way, the amount of work is small.

Per the manifest schema, Label has to come *after* Group in a CustomTab
element.
@jthake jthake merged commit e7933e9 into OfficeDev:master Jan 26, 2016
@jasonjoh jasonjoh deleted the commands branch February 3, 2016 16:30
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 98.752% when pulling 350ca7b on jasonjoh:commands into 83adab4 on OfficeDev:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants