Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

smashwilson
Copy link
Contributor

@smashwilson smashwilson commented Apr 23, 2018

Another bit of ambient cleanup work. This is me trying to get our PaneItem and opener management consistent and React-y. If we like it, I should be able to port over all of our existing PaneItems pretty quickly, as we don't have that many.

Here's a spoiler of what I'm building toward:

render() {
  return (
    <PaneItem
      workspace={this.props.workspace}
      uriPattern='atom-github://something/root/{filePath...}?q={search}'>
      {({itemHolder, params}) => (
        <ItemComponent
          ref={itemHolder.setter}
          filePath={params.filePath}
          search={params.search}
          another='prop'
        />
      )}
    </PaneItem>
  )
}

Fixes #1405.

Pane items to convert

  • FilePatchController
  • IssueishPaneItem
  • GitTimingsView

@smashwilson
Copy link
Contributor Author

@kuychaco @annthurium: I'm curious if you two like the API I've come up with here, and if you think it's a net benefit from the way that we used to manage pane items.

For reference, here's the new style:

The old style had a few different variants:

@smashwilson smashwilson requested review from annthurium and removed request for BinaryMuse April 26, 2018 02:57
@smashwilson
Copy link
Contributor Author

😆 Sorry about that, @BinaryMuse. I'm guessing I was overzealously clicking on suggested reviewers?

import url from 'url';

/**
* Match and capture parts of a URI, like a specialized dialect of regular expression. This is used by PaneItem to

Choose a reason for hiding this comment

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

yay documentation!!

@annthurium
Copy link

looks good to me!

* </PaneItem>
* );
* }
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

👍👍

Copy link
Contributor

@kuychaco kuychaco left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for doing this @smashwilson

@smashwilson
Copy link
Contributor Author

Cool, thanks for the sanity check 😄

@smashwilson smashwilson merged commit 3fab272 into master Apr 28, 2018
@smashwilson smashwilson deleted the aw/pane-item-pain branch April 28, 2018 00:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants