Skip to content

Conversation

@willlarche
Copy link
Contributor

@willlarche willlarche commented Sep 24, 2020

Description

The update to icons #61778 introduced a bug in the generated api doc.

This fixes the script so it uses the correct web font for the new icon styles and it updates the styling of the dart doc both to support the new classes and to remove unused styles.

Since the keys for the web font included the styling suffix, there was whitespace after the icon. That's been fixed.

Before:
Screen Shot 2020-09-23 at 8 22 40 PM

After:
Screen Shot 2020-09-23 at 8 22 13 PM

Related Issues

Closes #66407

Tests

I added the following tests:

I added no tests. I don't think we have those for the api doc site.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • [ x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x ] I signed the CLA.
  • [x ] I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • [ x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x ] I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • [ x] The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • [x ] I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

…cons. All the 4 icon styles we support have to be pulled from 4 individual web fonts when we show the html page for api docs.
@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Sep 24, 2020
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@willlarche willlarche changed the title [Docs] [Material] Fix font api docs [Docs] [Material] Fix Icons api docs Sep 24, 2020
Copy link
Contributor

@JoseAlba JoseAlba left a comment

Choose a reason for hiding this comment

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

██╗      ██████╗ ████████╗███╗   ███╗
██║     ██╔════╝ ╚══██╔══╝████╗ ████║
██║     ██║  ███╗   ██║   ██╔████╔██║
██║     ██║   ██║   ██║   ██║╚██╔╝██║
███████╗╚██████╔╝   ██║   ██║ ╚═╝ ██║
╚══════╝ ╚═════╝    ╚═╝   ╚═╝     ╚═╝

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

@willlarche willlarche merged commit ee6d4c6 into flutter:master Sep 24, 2020
@willlarche willlarche deleted the font-api-docs branch September 25, 2020 01:33
@pcsosinski
Copy link

removing CP label from PR in favor of issue (#66407)

christopherfujino pushed a commit to chris-forks/flutter that referenced this pull request Oct 7, 2020
* [Docs] [Icons] Updating dart doc styles to recognize more styles of icons. All the 4 icon styles we support have to be pulled from 4 individual web fonts when we show the html page for api docs.

* Better documentation and that special awful case.

* Couple more places to remove.
christopherfujino added a commit that referenced this pull request Oct 8, 2020
* [Docs] [Material] Fix Icons api docs (#66508)

* [Docs] [Icons] Updating dart doc styles to recognize more styles of icons. All the 4 icon styles we support have to be pulled from 4 individual web fonts when we show the html page for api docs.

* Better documentation and that special awful case.

* Couple more places to remove.

* Add back the autovalidate class property (#66267)

* Add back autovalidate property

* Add autovalidate property back (include tests)

* Fix typos

* One more test :)

* [flutter_tools] prevent running analyze-size with split-debug-info (#66983)

Running a build command with split debug info and analyze size causes a crash in the snapshot analysis library. Disable the combination of these two flags.

Fixes #66962

* fix invocatiion of non-existent function after cherry pick

* update engine revision

Co-authored-by: Will Larche <[email protected]>
Co-authored-by: Pedro Massango <[email protected]>
Co-authored-by: Jonah Williams <[email protected]>
willlockwood pushed a commit to willlockwood/flutter that referenced this pull request Dec 25, 2020
* [Docs] [Material] Fix Icons api docs (flutter#66508)

* [Docs] [Icons] Updating dart doc styles to recognize more styles of icons. All the 4 icon styles we support have to be pulled from 4 individual web fonts when we show the html page for api docs.

* Better documentation and that special awful case.

* Couple more places to remove.

* Add back the autovalidate class property (flutter#66267)

* Add back autovalidate property

* Add autovalidate property back (include tests)

* Fix typos

* One more test :)

* [flutter_tools] prevent running analyze-size with split-debug-info (flutter#66983)

Running a build command with split debug info and analyze size causes a crash in the snapshot analysis library. Disable the combination of these two flags.

Fixes flutter#66962

* fix invocatiion of non-existent function after cherry pick

* update engine revision

Co-authored-by: Will Larche <[email protected]>
Co-authored-by: Pedro Massango <[email protected]>
Co-authored-by: Jonah Williams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. 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.

[Docs] [Icons] Icons API page doesn't respect new icons

6 participants