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.

Sign up for GitHub

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

Jump to bottom

Use ItemSource to reduce code duplication #1835

Closed
YuPeiHenry opened this issue Mar 30, 2019 · 1 comment
Closed

Use ItemSource to reduce code duplication #1835

YuPeiHenry opened this issue Mar 30, 2019 · 1 comment
Labels
a-CodeQuality f-ColorsLab p.Low s.ToInvestigate Requires investigation

Comments

@YuPeiHenry
Copy link
Contributor

YuPeiHenry commented Mar 30, 2019 •
edited

Code quality

Use BindingSource to reduce code duplication

Description of Feature

Currently code is duplicated for each color palette in ColorsLab. See link

Proposal

Use .ItemSource to populate and auto-update each item.

Example of an application of .ItemSource for similar purposes: example
@YuPeiHenry YuPeiHenry changed the title Use BindingSource to reduce code duplication Use ItemSource to reduce code duplication Apr 3, 2019
@YuPeiHenry YuPeiHenry mentioned this issue Apr 11, 2019
Merged
blewjy pushed a commit that referenced this issue Apr 18, 2019
@YuPeiHenry @blewjy
#1835 Use ItemSource to reduce Code Duplication (#1866)
344d479 
Fixes #1835

Ready for Review. Ran ColorsLab test on PPT 2016.
**Outline of Solution**
The current implementation uses `ObservableCollection` to store the colors. It is possible to further reduce duplication in the xaml file (using ItemSource), but maintainers won't be able to see the actual panel being reflected in Visual Studio.
@YuPeiHenry YuPeiHenry reopened this Apr 19, 2019
@YuPeiHenry
Copy link
Contributor Author

The arrays used in ColorsLab have been removed in the previous PR referencing the issue. Now, further reduction can be done by adding WPF elements programmatically to ItemSource using a loop. The downside is that maintainers will not be able to see how the actual pane looks without starting PowerPoint,

@YuPeiHenry YuPeiHenry added the s.ToInvestigate Requires investigation label Apr 19, 2019
@yuhongherald yuhongherald mentioned this issue Jul 4, 2019
Merged
blewjy pushed a commit that referenced this issue Jul 11, 2019
@yuhongherald @blewjy
#1835 use item source to reduce code duplication (#1952)
8612aad 
* display seems to be working

* use itemsource for recent and favorite colors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-CodeQuality f-ColorsLab p.Low s.ToInvestigate Requires investigation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yuhongherald @YuPeiHenry