-
Notifications
You must be signed in to change notification settings - Fork 250
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
Add more classes to the Public API #717
Conversation
{ | ||
return null; | ||
} | ||
|
||
/// <summary> | ||
/// The page media box. | ||
/// </summary> | ||
public MediaBox MediaBox { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these property setters be internal to stop consumers messing with them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well spotted, now updated
@BobLd on |
@EliotJones good point on the resource store instance, I'll look into it and create another PR for that. Let's keep this one simple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As discussed in #656.
I've done the changes in several commits - that might help the review. I'll squash them when merging.
I've also update
NamedDestinations
namespace to match folder structure. Feel free to let me know if that should not the case.For the moment,
IPageFactory
stays internal, mainly becauseInternalParsingOptions
is. Same forIResourceStore
(and relatedXObjectFactory
).@EliotJones what's your view on
InternalParsingOptions
? I don't want to make it public but we will need to find a way to makeIResourceStore
andIResourceStore
public