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

Dismiss the PDF view controller when the view component is unmounted to avoid orphan popovers #280

Merged
merged 2 commits into from
Aug 28, 2019

Conversation

radazzouz
Copy link
Contributor

@radazzouz radazzouz commented Aug 27, 2019

Solves #277


Details

How to Reproduce:

  • Modify Catalog.ios.js like this:
class ConfiguredPDFViewComponent extends Component {
+ componentDidMount = () => {
+   setTimeout(() => {
+     this.props.navigation.navigate("Catalog");
+   }, 10000);
+ };
  render() {
    return (
      <View style={{ flex: 1 }}>
        <PSPDFKitView
          document={"PDFs/Annual Report.pdf"}
          configuration={{
            backgroundColor: processColor("lightgrey"),
            showThumbnailBar: "scrubberBar",
            showDocumentLabel: false,
            useParentNavigationBar: false,
            allowToolbarTitleChange: false
          }}
          toolbarTitle={"Custom Title"}
+         leftBarButtonItems={["settingsButtonItem"]}
          style={{ flex: 1, color: pspdfkitColor }}
        />
      </View>
    );
  }
}
  • Open the "PDF View Component" example.
  • Tap on the settings left bar button item and wait a few seconds for the navigation (for the component to be unmounted).

Expected:

The popover should disappear.

Actual:

The popover is still there even if the pdf view controller has been dismissed.

Before After
before after

Acceptance Criteria

  • Fix the issue.
  • When approved, right before merging, rebase with master and increment the package version in package.json, package-lock.json, and samples/Catalog/package.json (see example commit: 1bf805f).
  • Create a new release (and tag) with the new package version (see https://github.com/PSPDFKit/react-native/releases).

Copy link
Contributor

@steviki steviki left a comment

Choose a reason for hiding this comment

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

LGTM!

@radazzouz radazzouz merged commit eb2a80f into master Aug 28, 2019
@radazzouz radazzouz deleted the rad/dismiss-when-the-view-component-is-unmounted branch August 28, 2019 11:39
@@ -38,6 +38,11 @@ - (instancetype)initWithFrame:(CGRect)frame {
return self;
}

- (void)removeFromSuperview {
[self.pdfController dismissViewControllerAnimated:NO completion:NULL];
Copy link
Contributor

Choose a reason for hiding this comment

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

@radazzouz Please add a comment to that, so we know what this was for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Done in #287

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.

3 participants