Skip to content

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Dec 3, 2019

Fixes ~200 issues currently with docs.

As it is this may have to be split up.

Addresses #31931

I basically started working from top down of dartdoc warnings/errors, although some I did en masse (e.g. DiagnosticNode -> DiagnosticsNode, SemanticNode -> SemanticsNode) and some I grabbed because they were just close by in the file.

@fluttergithubbot fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Dec 3, 2019
@dnfield
Copy link
Contributor Author

dnfield commented Dec 3, 2019

(There are still >600 issues remaining by local estimates)

@dnfield dnfield added the d: api docs Issues with https://api.flutter.dev/ label Dec 3, 2019

@override
String toString() => '$runtimeType(value: begin)';
String toString() => '$runtimeType(value: $begin)';
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 🙁

/// Mandatory string to add after the properties of a node regardless of
/// whether the node has any properties.
///
/// See [headerLineTextConfiguration] for an example of using this field to
Copy link
Contributor

Choose a reason for hiding this comment

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

singleLineTextConfiguration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That field sets this to '' - doesn't seem like a great example :\

/// Widgets and render objects at lower layers that try to emulate the
/// underlying platform can depend on [defaultTargetPlatform] directly. The
/// [dart.io.Platform] object should only be used directly when it's critical to
/// [platform.Platform] object should only be used directly when it's critical to
Copy link
Contributor

Choose a reason for hiding this comment

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

Not [dart:io.Platform]?

///
/// We occasionally synthesize PointerEvents that aren't exact translations
/// of [ui.PointerData] from the engine to cover small cross-OS discrepancies
/// of [PointerData] from the engine to cover small cross-OS discrepancies
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this not have to be [dart:ui.PointerData]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think either one would work - this works because we're importing it in this file.

/// This function must always return values in the range 0.0 to 1.0 given a
/// pressure that is between the minimum and maximum pressures. It may return
/// [double.NaN] for values that it does not want to support.
/// `double.NaN` for values that it does not want to support.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it'd link it this was [dart:core.double.NaN]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No luck. I think this is probably a dartdoc bug but I'm also not sure how important it is we link to double.NaN here.

///
/// See also:
///
/// * [ComputeNotch] a function used for creating a notch in a shape.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe link to [NotchedShape]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

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

lgtm
for the widget_inspector typo fixes.

/// 1. [localeListResolutionCallback] is attempted first.
/// 2. [localeResolutionCallback] is attempted second.
/// 3. Flutter's [WidgetsApp.basicLocaleListResolution] algorithm is attempted last.
/// 3. Flutter's basic resolution algorithm is attempted last.
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the basic resolution algorithm documented?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's documented in supportedLocales - maybe link to there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

///
/// The order of the list matters. The default locale resolution algorithm,
/// [basicLocaleListResolution], attempts to match by the following priority:
/// `basicLocaleListResolution`, attempts to match by the following priority:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a symbol at all now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's private API. Reworded.

/// 4. [Locale.languageCode] only
/// 6. [Locale.countryCode] only when all [preferredLocales] fail to match
/// 5. returns [supportedLocales.first] as a fallback
/// 5. [Locale.countryCode] only when all preferred locales fail to match
Copy link
Contributor

Choose a reason for hiding this comment

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

Just make these all 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

For the material and cupertino library changes

/// after it is resolved against a [BuildContext] that:
/// - has a [CupertinoTheme] whose [brightness] is [PlatformBrightness.light],
/// or a [MediaQuery] whose [MediaQueryData.platformBrightness] is [PlatformBrightness.light].
/// - has a [CupertinoTheme] whose [brightness] is [Brightness.light],
Copy link
Contributor

Choose a reason for hiding this comment

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

CC @LongCatIsLooong , looks like a systematic error

///
/// See also:
///
/// * [ComputeNotch] a function used for creating a notch in a shape.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice discard.

Copy link
Contributor

@tvolkert tvolkert left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM on gestures.

///
/// This triggers when the pointer stops contacting the device after the 2nd tap,
/// immediately after [onDoubleTapUp].
/// immediately after the second [GestureTapUpCallback].
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the 2 lines to:

  /// This triggers when the pointer stops contacting the device after the 2nd tap.

The onDoubleTapUp is an proposed API that was eventually removed. Thanks for spotting it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM on gestures

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • This pull request has changes requested. Please resolve those before re-applying the label.

@dnfield dnfield merged commit b61dec7 into flutter:master Dec 3, 2019
@dnfield dnfield deleted the doc_fixup branch December 3, 2019 21:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) d: api docs Issues with https://api.flutter.dev/ f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants