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

1st prototype of Texture debugger #339

Closed
wants to merge 3 commits into from
Closed

Conversation

nguyenhuy
Copy link
Member

@nguyenhuy nguyenhuy commented Jun 6, 2017

Author: Thanh Huy Nguyen

  • Client apps connect to Chrome DevTools via ponyd server. Setup instructions coming.
  • Users can highlight display nodes as well as layout specs. ASDKGram is updated to use the debugger. See video here.
  • Style and other useful properties will be exposed later on.

This PR depends on #337. Please review #337 first.

@ghost
Copy link

ghost commented Jun 6, 2017

🚫 CI failed with log

@ghost
Copy link

ghost commented Jun 8, 2017

🚫 CI failed with log

nguyenhuy and others added 3 commits June 8, 2017 14:35
- Client apps connect to Chrome DevTools via ponyd server. Setup instructions coming.
- Users can highlight display nodes as well as layout specs.
- Style and other useful properties will be exposed later on.
@ghost
Copy link

ghost commented Jun 8, 2017

1 Warning
⚠️ This is a big PR, please consider splitting it up to ease code review.

Generated by 🚫 Danger

@ghost
Copy link

ghost commented Jun 8, 2017

🚫 CI failed with log

Copy link
Member

@garrettmoon garrettmoon left a comment

Choose a reason for hiding this comment

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

Awesome, so exciting!


- (CGRect)td_frameInWindow
{
return [self convertRect:self.bounds toLayer:nil];
Copy link
Member

Choose a reason for hiding this comment

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

I could be wrong, but I don't think this is actually correct for layers, only views? There's nothing in the documentation that it coverts to window for CALayer. In fact what it says is super opaque:

If you specify nil for the l parameter, this method returns the original rect with an origin added to the layer's frame's origin.

I don't know what origin is… perhaps it's based on the window…

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, you're right. Let me revisit this!

@@ -1,6 +1,8 @@
source 'https://github.com/CocoaPods/Specs.git'
platform :ios, '8.0'
target 'Sample' do
pod 'Texture/IGListKit', :path => '../..'
pod 'Texture/PINRemoteImage', :path => '../..'
pod 'PonyDebugger', :git => 'https://github.com/nguyenhuy/PonyDebugger.git', :branch => 'master'
Copy link
Member

Choose a reason for hiding this comment

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

Just to be clear, the endgame is not to have people point to a fork in the end?

Copy link
Member Author

@nguyenhuy nguyenhuy Jun 8, 2017

Choose a reason for hiding this comment

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

Once we completely fork PonyDebugger and publish a TextureDebugger pod, this will be unnecessary.

@nguyenhuy
Copy link
Member Author

CI is failing at pod lib lint step. That's because I'm using a forked version of PonyDebugger and I don't think we can declare a forked dependency in Podspec. I can send PRs to the original PonyDebugger repo, but given that the repo has very old PRs and seems to be unmaintained overall, I'm gonna bite the bullet and setup a new TextureDebugger pod...

@nguyenhuy
Copy link
Member Author

Closing this in favor of a separate repo for the debugger here.

@nguyenhuy nguyenhuy closed this Jun 21, 2017
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.

None yet

2 participants