Skip to content

Hide implementation of static constants #2657

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

Closed
guidezpl opened this issue May 25, 2021 · 13 comments · Fixed by #3398
Closed

Hide implementation of static constants #2657

guidezpl opened this issue May 25, 2021 · 13 comments · Fixed by #3398
Assignees
Labels
customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug

Comments

@guidezpl
Copy link
Contributor

The Icons class defines icons as IconData constants. This has lead a number of devs to use the implementation (e.g. IconData(57399, fontFamily: 'MaterialIcons')) rather than the interface Icons.airplane. This is a problem because all codepoints (e.g. 57399) change every time the font is updated.

Is there an option or annotation to hide the implementation in docs?

@srawlins
Copy link
Member

Do you have evidence that developers are copy/pasting from dartdoc rather than from the source (like clicking through in their IDE)?

Can the constructor be made private or @internal so that users are discouraged from using it?

@jcollins-g jcollins-g added the P2 A bug or feature request we're likely to work on label May 25, 2021
@jcollins-g
Copy link
Contributor

The way this looks now on the top level page, I could see how people might interpret the text string as an example.

Screen Shot 2021-05-25 at 3 08 25 PM

What if we moved the value onto the same line as the definition in the template? That might make it more obvious that it isn't intended as an example.

@jcollins-g
Copy link
Contributor

So the first line for a constant would be something like adb_outlined -> const IconData(60984, fontFamily: 'MaterialIcons'), all in the same font/weight.

@guidezpl
Copy link
Contributor Author

@srawlins I've lost count but it happens fairly regularly (e.g. flutter/flutter#83208). The IconData constructor can be used legitimately for other purposes (such as with one's own icon font). The @internal annotation is not suitable beause it generates can only be used within its package analysis warnings.

@jcollins-g that would be a slight improvement, but ideally I'd like to completely hide the constants' implentation from the API docs.

@jcollins-g
Copy link
Contributor

Another way to implement this is as a dartdoc tag. Something like @hideImplementations in the enum -- that would enable us to drop that tag in in places where the implementation is uninteresting or even distracting/leading users down the wrong path.

@jcollins-g jcollins-g added the type-enhancement A request for a change that isn't a bug label Jun 2, 2021
@guidezpl
Copy link
Contributor Author

guidezpl commented Jun 3, 2021

That would be ideal!

@jcollins-g jcollins-g added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Aug 30, 2021
@jcollins-g
Copy link
Contributor

This page turns out to have a lot of traffic in the Flutter docs, so finding a way to reduce misinterpretations here would have a high impact.

@guidezpl
Copy link
Contributor Author

guidezpl commented Nov 5, 2021

Hey, wondering if adding this tag is something I could do? How would I go about that?

@guidezpl
Copy link
Contributor Author

sorry to ask, but "any updates?"

@srawlins
Copy link
Member

srawlins commented Aug 4, 2022

Sorry, no updates.

@jcollins-g jcollins-g added the customer-flutter Issues originating from important to Flutter label Mar 24, 2023
@jcollins-g jcollins-g self-assigned this Apr 24, 2023
@guidezpl
Copy link
Contributor Author

guidezpl commented May 4, 2023

Thanks @jcollins-g! I wonder though, since we now rely on the single staticIconProvider annotation (e.g. https://github.com/guidezpl/flutter/blob/constrain-dialogs/packages/flutter/lib/src/material/icons.dart#L152) rather than insert thousands of individual annotations, to achieve the same result?

@jcollins-g
Copy link
Contributor

Thanks @jcollins-g! I wonder though, since we now rely on the single staticIconProvider annotation (e.g. https://github.com/guidezpl/flutter/blob/constrain-dialogs/packages/flutter/lib/src/material/icons.dart#L152) rather than insert thousands of individual annotations, to achieve the same result?

You shouldn't need thousands of annotations, you should only need a single annotation for the class containing the constants in question.

@guidezpl
Copy link
Contributor Author

guidezpl commented Jun 2, 2023

You're right, I don't know where I got that idea (:

auto-submit bot pushed a commit to flutter/flutter that referenced this issue Jun 8, 2023
…c pages (#128442)

This updates dartdoc to 6.3.0.  Release notes are available, here:

https://github.com/dart-lang/dartdoc/releases/tag/v6.3.0

Most important for Flutter are the reduction in the size of generated HTML files (dart-lang/dartdoc#3384) and a new dartdoc directive to hide constant implementations from indicated classes (dart-lang/dartdoc#3398), which fixes the longstanding issue (dart-lang/dartdoc#2657).

I've also added the api documentation zip to `.gitignore` and the `{@hideConstantImplementations}` dartdoc directive to the motivating example.  A screenshot:

![Screenshot 2023-06-07 at 9 54 58 AM](https://github.com/flutter/flutter/assets/14116827/1ad9c1f0-b224-462f-a8e3-706d9858f0d8)

I assert that this change to icons.dart should be test-exempt as existing tests cover whether or not dartdoc directives are recognized or are leaking into HTML, and the impact of adding the directive was tested in dart-lang/dartdoc#3398.
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 P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants