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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add About page to the settings window #103

Merged
merged 21 commits into from Oct 28, 2018

Conversation

Projects
None yet
2 participants
@ericcornelissen
Copy link
Contributor

ericcornelissen commented Oct 22, 2018

As per #98

preview

Architectural changes

I moved the UpdateService and NotificationService service to the App.Services package(?) and created interfaces for them as well. This resulted in the following changes:

Checking for updates As a consequence I moved the check for updates to `App.xaml.cs` which may have some unwanted consequences: 1) not checked for updates if the main/terminal window is not opened at startup, and 2) now checks for updates whenever a new main/terminal window (or tab?) is opened. However, I think it is possible to revert this change 馃
NotificationService I'm not sure how C# handles this, but note that the `NotificationService` is in both `App.xaml.cs` and `Program.cs`, I'm not sure what consequences this has...

Code style notes

  • I used comments in About.xaml and I noticed the existing code base had no comments for the page layout files
  • The update service returns new Version(0, 0, 0, 0); if it can't find the latest version online
  • The data for the About page is loaded in the Page class rather then the ViewModel (the reason for this is that I had trouble getting the page to show the asynchronously loaded latest version) Resolved with db01c51

Miscellaneous notes

  • The icon for the About tab is taken from Segoe MDL2 Assets rather then the custom font-file included in the project
  • Not quite sure about the <PackageReference>s in FluentTerminal.App.Services.csproj, but this was the only way I got things working 馃槄
  • I added a new version of the logo to the Assets/AppIcons folder for the About page, but I'm not sure if that is the correct location for it 馃
  • I had to use a little bit of a hack in SettingsPage.xaml.cs to make sure the tab indicator in the settings view works correctly when the about page shows, you can read more here

Hamburger menu

As already mentioned in #98 I did not manage to add a working menu item to the hamburger menu in the main/terminal window. You can find the code for my attempt right here Resolved with c4a04bc

@felixse

This comment has been minimized.

Copy link
Owner

felixse commented Oct 27, 2018

Looks good, thank you very much 馃憤
I added a commit to extend the hamburger menu

If you need any help with my comments just write me

@ericcornelissen

This comment has been minimized.

Copy link
Contributor Author

ericcornelissen commented Oct 27, 2018

Thanks for extending the hamburger menu 馃槉

@felixse felixse closed this Oct 28, 2018

@felixse felixse reopened this Oct 28, 2018

@felixse felixse merged commit 2f58ed6 into felixse:master Oct 28, 2018

1 check was pending

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details

@ericcornelissen ericcornelissen deleted the ericcornelissen:about branch Oct 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment