Skip to content

xFonts 2.0 - #1

Merged
manolosavi merged 61 commits into
manolosavi:masterfrom
chockenberry:master
Jun 7, 2020
Merged

xFonts 2.0#1
manolosavi merged 61 commits into
manolosavi:masterfrom
chockenberry:master

Conversation

@chockenberry

Copy link
Copy Markdown
Collaborator

These are all the changes that we'll be releasing as Fontcase on the App Store.

…ain view controller to a table view controller. UI cleanup.
Comment thread xFonts/HeaderView.m Outdated
self.statusLabel.text = [NSString stringWithFormat:@"%ld font%s added, %ld need%s install", addedCount, (addedCount == 1 ? "" : "s"), installCount, (installCount == 1 ? "s" : "")];
}
else {
self.statusLabel.text = [NSString stringWithFormat:@"%ld fonts added", addedCount];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.statusLabel.text = [NSString stringWithFormat:@"%ld fonts added", addedCount];
self.statusLabel.text = [NSString stringWithFormat:@"%ld font%s added", addedCount, (addedCount == 1 ? "" : "s")];

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Comment thread xFonts/HeaderView.m Outdated
{
//DebugLog(@"%s called", __PRETTY_FUNCTION__);
if (previousTraitCollection.userInterfaceStyle != self.traitCollection.userInterfaceStyle) {
//[self loadHelpMarkdown];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Test code that was forgotten :-)

Comment thread xFonts/HelpViewController.m Outdated

UIFont *baseFont = [UIFont preferredFontForTextStyle:UIFontTextStyleBody];

#if 1

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this if/else still needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. The new code is simpler and more reliable.

Comment thread xFonts/ViewController.h Outdated
Comment on lines +12 to +13
//#import "PresentationBlurAnimator.h"
//#import "DismissBlurAnimator.h"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//#import "PresentationBlurAnimator.h"
//#import "DismissBlurAnimator.h"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup!

Comment thread xFonts/ViewController.h Outdated
Comment on lines +18 to +23
//
//@property (nonatomic, weak) IBOutlet UITableView *tableView;
//@property (nonatomic, weak) IBOutlet UIBarButtonItem *installButton;
//@property (nonatomic, weak) IBOutlet UIBarButtonItem *addButton;
//
//@property (nonatomic, strong) NSArray<FontInfo *> *fonts;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//
//@property (nonatomic, weak) IBOutlet UITableView *tableView;
//@property (nonatomic, weak) IBOutlet UIBarButtonItem *installButton;
//@property (nonatomic, weak) IBOutlet UIBarButtonItem *addButton;
//
//@property (nonatomic, strong) NSArray<FontInfo *> *fonts;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup!

Comment thread xFonts/ViewController.m Outdated
{
[super viewDidAppear:animated];

#if 1

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this if/else still needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep this one in - helpful to test the overlay without clearing out all the fonts.

Comment thread xFonts/ViewController.m Outdated
NSString *message = [NSString stringWithFormat:@"The mobile configuration profile could not be created.\n\nThe error was '%@'", error.localizedDescription];
UIAlertController *alertController = [UIAlertController alertControllerWithTitle:@"Install Error" message:message preferredStyle:UIAlertControllerStyleAlert];
alertController.view.tintColor = self.view.tintColor;
[alertController addAction:[UIAlertAction actionWithTitle:@"OK" style:UIAlertActionStyleCancel handler:nil]];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be this instead?

Suggested change
[alertController addAction:[UIAlertAction actionWithTitle:@"OK" style:UIAlertActionStyleCancel handler:nil]];
[alertController addAction:[UIAlertAction actionWithTitle:@"OK" style:UIAlertActionStyleDefault handler:nil]];

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Comment thread xFonts/ViewController.m
NSString *profile = [NSString stringWithFormat:@"<?xml version=\"1.0\" encoding=\"UTF-8\"?><!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\"><plist version=\"1.0\"><dict><key>PayloadType</key><string>Configuration</string><key>PayloadVersion</key><integer>1</integer><key>PayloadDisplayName</key><string>xFonts (%@)</string><key>PayloadIdentifier</key><string>xFonts %@</string><key>PayloadUUID</key><string>%@</string><key>PayloadContent</key><array>%@</array></dict></plist>", title, NSUUID.UUID.UUIDString, NSUUID.UUID.UUIDString, fonts];
NSString *profile = [NSString stringWithFormat:profilePayloadTemplate, productName, productName, NSUUID.UUID.UUIDString, fontsPayload];

NSURL *URL = [FontInfo.storageURL URLByAppendingPathComponent:@"xFonts.mobileconfig"];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be renamed too?

Suggested change
NSURL *URL = [FontInfo.storageURL URLByAppendingPathComponent:@"xFonts.mobileconfig"];
NSURL *URL = [FontInfo.storageURL URLByAppendingPathComponent:@"Fontcase.mobileconfig"];

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that, but it's never seen by the customer. It's kind of like the bundle ID (that uses xFonts) so I'd say leave it as-is.

Comment thread xFonts/ViewController.m
NSString *fileAtPath = [self pathForFile:fileName];
if (![[NSFileManager defaultManager] fileExistsAtPath:fileAtPath]) {
[[NSFileManager defaultManager] createFileAtPath:fileAtPath contents:nil attributes:nil];
- (void)applicationDidBecomeActive:(UIApplication *)application

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks right to me but when running the TestFlight im not seeing it update when i come back to the app after installing. it still shows the font as "need to install" until i kill the app, and then it does show as "installed"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a bug with the Font Manager and fonts that are registered in the process scope. It sometimes doesn't refresh what's available until the process restarts. I'd like to fix it, of course, but it may be something we have to live with...

@chockenberry

Copy link
Copy Markdown
Collaborator Author

Thanks for going through these - you found some good stuff!

Comment thread xFonts/Help.md Outdated
Open the _Settings_ app and you’ll see a _Profile Downloaded_ button above the Airplane Mode switch. (If not, navigate to _General_ > _Profiles_ > _Downloaded Profile_ > _$(PRODUCT_NAME) Installation_.) After tapping the button, the first thing you’ll see is “Not Signed” highlighted in red.

This is another instance where you need to trust the source of the profile and its contents. Since the profile was generated on your device, there’s no risk that it was modified in transit. But unfortunately theres no way to sign a profile locally without exposing a secret: the private key would have to be in the app’s source code or resources.
This is another instance where you need to trust the source of the profile and its contents. Since the profile was generated on your device, there’s no risk that it was modified in transit. This also means there's no way to sign a profile locally without exposing a secret: the private key would have to be in the app’s source code or resources.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nit: not sure if intentional or not but i think maybe we should keep using instead of '

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'll fix this in the next PR.

@chockenberry

Copy link
Copy Markdown
Collaborator Author

Famous last words: ready for release 😄

@chockenberry

Copy link
Copy Markdown
Collaborator Author

Added App Store metadata, including screenshots. See above 😄

@manolosavi manolosavi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think it looks good, lmk how you wanna proceed here

@manolosavi
manolosavi merged commit 95c20ce into manolosavi:master Jun 7, 2020
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.

2 participants