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

Some classes not hyperlinked #1152

Closed
Hixie opened this issue Apr 21, 2016 · 13 comments · Fixed by #1207
Closed

Some classes not hyperlinked #1152

Hixie opened this issue Apr 21, 2016 · 13 comments · Fixed by #1207
Assignees
Labels
customer-flutter Issues originating from important to Flutter P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@Hixie
Copy link
Contributor

Hixie commented Apr 21, 2016

If you look at: http://docs.flutter.io/flutter/gestures/GestureBinding-class.html

It says "Implements: HitTestable, HitTestDispatcher, HitTestTarget", but those classes aren't hyperlinked. This is weird, since they are also listed in the sidebar, and there they are hyperlinks.

Source: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/gestures/binding.dart

@Hixie
Copy link
Contributor Author

Hixie commented May 5, 2016

http://docs.flutter.io/flutter/material/ThemeData/ThemeData.html shows this also. All the properties mentioned in the body of the constructor description should be hyperlinks but aren't.

Source: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/material/theme_data.dart

@Hixie
Copy link
Contributor Author

Hixie commented May 6, 2016

Here's a particularly bad case: http://docs.flutter.io/flutter/widgets/CustomPainter/paint.html

Notice how Canvas.drawImage gets hyperlinked but Canvas.drawImageRect doesn't. Both are exported, both exist, both are in the docs. What the heck?

Source for CustomPainter:
https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/proxy_box.dart

Source for Canvas:
https://github.com/flutter/engine/blob/master/sky/engine/core/dart/painting.dart

@Hixie
Copy link
Contributor Author

Hixie commented May 6, 2016

cc @devoncarew

@devoncarew devoncarew added the customer-flutter Issues originating from important to Flutter label May 6, 2016
@keertip
Copy link
Collaborator

keertip commented May 6, 2016

@Hixie, can you provide a link to the source code?

@Hixie
Copy link
Contributor Author

Hixie commented May 6, 2016

The script that generates the docs is:
https://github.com/flutter/flutter/blob/master/infra/docs.sh

I've updated the comments on this bug with links to the files that contain the classes in question.

@devoncarew
Copy link
Member

devoncarew commented May 6, 2016

Perhaps something to do with the markdown resolver? The items the get resolved here:

screen shot 2016-05-06 at 12 38 39 pm

are on the first line of the markdown list; the ones that don't are on the following lines.

https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/proxy_box.dart#L1424

@Hixie Hixie added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) P0 A serious issue requiring immediate resolution labels Jun 16, 2016
@eseidel
Copy link

eseidel commented Jun 23, 2016

This diff will make the resolver work:

diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart
index a227f1a..72651a2 100644
--- a/packages/flutter/lib/src/rendering/proxy_box.dart
+++ b/packages/flutter/lib/src/rendering/proxy_box.dart
@@ -1476,7 +1476,7 @@ abstract class CustomPainter {
   ///    paint delegate, giving it the new [ImageInfo] object.
   ///
   /// 3. In your delegate's [paint] method, call the [Canvas.drawImage],
-  ///    [Canvas.drawImageRect], or [Canvas.drawImageNine] methods to paint the
+  ///   [Canvas.drawImageRect], or [Canvas.drawImageNine] methods to paint the
   ///    [ImageInfo.image] object, applying the [ImageInfo.scale] value to
   ///    obtain the correct rendering size.
   void paint(Canvas canvas, Size size);

I think this might be the bug:
https://github.com/dart-lang/markdown/blob/157b8403ef065d25012b50c81311d9c262484c6d/lib/src/document.dart#L39

:)

@eseidel
Copy link

eseidel commented Jun 23, 2016

@kevmoo might know if my analysis above re: the possible package:markdown bug is correct. :) Probably my next step should be to write a unittest for markdown and see.

@kevmoo
Copy link
Member

kevmoo commented Jun 23, 2016

@eseidel plausible. A unittest would be nice.

@keertip keertip added this to the hackathon milestone Jul 20, 2016
@devoncarew devoncarew self-assigned this Jul 20, 2016
@devoncarew
Copy link
Member

Some of the earlier items in this bug are fixed; I'll take a look at the markdown related one.

@Hixie
Copy link
Contributor Author

Hixie commented Jul 26, 2016

I still see this e.g. in:
https://docs.flutter.io/flutter/material/OverlayEntry-class.html

(Draggable and AnimatedPositioned aren't hyperlinked, despite being listed in the index on the same page)

@Hixie Hixie reopened this Jul 26, 2016
@sethladd
Copy link
Contributor

What version of dartdoc did we use to build the docs.flutter.io? There used to be a comment in HTML files with the version number, but I can't find it?

@devoncarew
Copy link
Member

The version is written into index.html; flutter docs are built w/ the last published version of dartdoc (these generated docs are up-to-date w/ the tool).

From looking at the source, the entries that we can't resolve really aren't resolvable w/ the file's imports. We'd need to either import the draggable and animatedpositioned libs into widgets/overlay.dart, or implement something like #1153. Let's track the fix in #1153.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-flutter Issues originating from important to Flutter P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants