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

Wpf Example Change TabControl to ItemsSource Take #2 #327

Conversation

amaitland
Copy link
Member

Initial set of changes to convert the current CefSharp.Wpf.Example to a Tabbed Control with ItemsSource.
Second take at this set of changes, this is the basic set to start with.

View -> ViewModel mapping is now handled by DataTemplate
Imported TabControlEx control to prevent tabs from being reloaded each time they've viewed
Added close button to each tab
Added some basic styling to TabItems
Add ViewSource command directly to WebView and IWpfWebBrowser
Added ctrl->t key cord to open new browser tab
Added ctrl->w key cord to close current browser tab
Added copyright disclaimer to header of each code file

View -> ViewModel mapping is now handled by DataTemplate
Imported TabControlEx control to prevent tabs from being reloaded each time they've viewed
Added close button to each tab
Added some basic styling to TabItems
Add ViewSource command directly to WebView and IWpfWebBrowser
Added ctrl->t key cord to open new browser tab
Added ctrl->w key cord to close current browser tab
Added copyright disclaimer to header of each code file

Signed-off-by: amaitland <alex@maitlandcomputing.com.au>
@amaitland amaitland changed the title Changed TabControl to use ItemsSource Take #2 Wpf Example Change TabControl to ItemsSource Take #2 Apr 16, 2014
@perlun
Copy link
Member

perlun commented Apr 16, 2014

Hi,

I would like this commit (17 files changed...) to be split in even smaller parts. I.e. "Added Ctrl-T and Ctrl-W shortcuts" could very well be a pull request of its own.

So, sorry to have to reject your fine work again (really!) but please, try splitting it even further. Generally, 17 files changed within one single commit is usually never a good thing. (Perhaps it would have been "kind of OK" if this was 8 different commits, one for each line in your list.)

@amaitland
Copy link
Member Author

Hi @perlun,

I've created a few separate PR's for the noise changes like adding the license disclaimer which would greatly reduce the number of files changed. Also another request to add a ViewSourceCommand, so that I can limit the changes to the Wpf.Example project.

As for the other items, I believe it's important to keep at least this minimal set of changes together, items like Adding Ctrl-T and Ctrl-W shortcuts, the command binding definition is the same, however the implementation is totally different when switching over to an ItemsSource based approach.

If I create another PR with a number of smaller more concise commits meet your remaining requirements?

@perlun
Copy link
Member

perlun commented Apr 18, 2014

Hi,

Yes, I see your point. Split it into a PR with smaller, more concise commits and I think I'll be more willing to accept it.

@amaitland amaitland closed this Apr 30, 2014
@amaitland amaitland deleted the feature/wpf-tabcontrol-itemssource branch May 13, 2014 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants