Various improvements

[akrantz]

I made some various improvements. Primarily I wanted the TextInsertion component to use a prop to pass the insertText behavior and better align how the code can work across hosts. The insertText() code is in the excel.ts, powerpoint.ts, etc., as well as in the taskpane.ts which works across hosts. So convertToSingleHost script can just rename {host}.ts to taskpane.ts which is how it works in the non-React templates. I aligned the file names to use the same naming for consistency.

By having the App component control the app UI via how the components are rendered and the properties for those components, I think it's more straight-forward. I also passed the host as a prop to the App component so that logic could be used to only render particular components for particular hosts, but I didn't actually make an example there.

For example, this could allow a component to be shown only for the Excel host: (host === Office.HostType.Excel) ? (<ExcelComponent/>)

Partly these changes were driving by sample code when working with a customer which is targetting multiple hosts for their add-in, and rather than making these types of changes for the sample code, I wanted to contribute it back to the React template itself.

I changed the insertText() code for PowerPoint. It's better to use shapeCollection.addTextBox(). I would like to apply this back to the basic taskpane template.

1. Do these changes impact any npm scripts commands (in package.json)? (e.g., running 'npm run start') No

2. Do these changes impact VS Code debugging options (launch.json)? No. I validated in Word, Excel, and PowerPoint, but had problems for Outlook, as it wouldn't sideload. I think something might be broken there independent of these changes.

3. Do these changes impact template output? (e.g., add/remove file, update file location, update file contents) Yes, some files were renamed.

4. Do these changes impact documentation? (e.g., a tutorial on https://docs.microsoft.com/en-us/office/dev/add-ins/overview/office-add-ins) No, I don't think it should.

If you answered yes to any of these please do the following:

Include 'Rick-Kirkham' in the review Make sure the README file is correct

Validation/testing performed:

Describe manual testing done. 

[Rick-Kirkham]

@akrantz Thanks. I won't have bandwidth to review this until April, probably well into April.

[akrantz]

@darrelmiller @Rick-Kirkham When you have time, it would be good to get feedback on this PR.

[Rick-Kirkham]

@darrelmiller @Rick-Kirkham When you have time, it would be good to get feedback on this PR.

The thing that made me postpone looking at this till April was itself postponed till mid-April. But I'll try to take a look today or Monday.

[akrantz]

Thanks Ricky. I added the host prop to the App component because I thought it was a useful pattern. But I can remove it since I had ended up not using it in the end.

From: Rick Kirkham @.> Sent: Friday, April 5, 2024 2:39 PM To: OfficeDev/Office-Addin-TaskPane-React @.> Cc: Adam Krantz @.>; Mention @.> Subject: Re: [OfficeDev/Office-Addin-TaskPane-React] Various improvements (PR #152)

@Rick-Kirkham requested changes on this pull request.

@akrantzhttps://github.com/akrantz You did a great job! Thanks! Just one concern. In the outputted projects, the AppProps interface in App.tsx has a host property that is passed in from index.tsx. But this value is never used. Can we remove it? If it is needed for the tests, then can we have the conversion script remove it from App.tsx and index.tsx?

---

In src/taskpane/components/App.tsxhttps://github.com/OfficeDev/Office-Addin-TaskPane-React/pull/152#discussion_r1554327333:

interface AppProps {

• host: Office.HostType;

This host prop is never used. Can we remove it from the interface?

---

In src/taskpane/index.tsxhttps://github.com/OfficeDev/Office-Addin-TaskPane-React/pull/152#discussion_r1554327702:

 <FluentProvider theme={webLightTheme}>

This host prop is never used. Can we remove it?

- Reply to this email directly, view it on GitHubhttps://github.com/OfficeDev/Office-Addin-TaskPane-React/pull/152#pullrequestreview-1984224467, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAFU6IIAC36WQOKBTXB23F3Y34KY5AVCNFSM6AAAAABEYYDYWWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOBUGIZDINBWG4. You are receiving this because you were mentioned.Message ID: @.**@.>>

[akrantz]

I've merged in the latest master branch, resolved merge conflicts, fixed the end-to-end test for PowerPoint, and validated that for WXP, I can convert to single host, run the dev server, and debug. So things should all be good now. @Rick-Kirkham @millerds

[Rick-Kirkham]

OK. The tests pass locally now, so I'm approving.

[akrantz]

@millerds Once you approve, feel free to go ahead and merge.

[millerds]

@millerds Once you approve, feel free to go ahead and merge.

Looks like there are linter errors in the pipelines.

[akrantz]

@millerds Fixed the prettier errors.

[millerds]

pipeline tests are failing.

[akrantz]

@millerds I fixed the mocks so the PowerPoint unit test should pass now.