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

Support icon previews in editors via the analysis server #47667

Open
DanTup opened this issue Nov 10, 2021 · 12 comments
Open

Support icon previews in editors via the analysis server #47667

DanTup opened this issue Nov 10, 2021 · 12 comments
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DanTup
Copy link
Collaborator

DanTup commented Nov 10, 2021

Similar to how the analysis server will provide color previews to editors, it would be nice if icon paths for previews could also be provided.

Right now Dart-Code ships a full copy of the Flutter's Material/Cupertino icons in the extension and adds gutter icons based on text names. This doesn't scale very well to third party packages, needs duplicating in each editor using the LSP server, and doesn't version with the package/SDKs (all users get the same icon previews regardless of their version). It also makes the extension quite large (right now, ~99% of the Dart-Code extension file is icon previews).

@stevemessick has done some work on generating the previews from a font file in IntelliJ (flutter/flutter-intellij#5504, flutter/flutter-intellij#5595) but it has better access to things like ASTs than LSP-based editors so it would be better implemented in the server for those.

@mraleph mraleph added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Nov 10, 2021
@srawlins srawlins added analyzer-server P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug labels Nov 15, 2021
@DanTup
Copy link
Collaborator Author

DanTup commented Apr 28, 2022

@jacob314 @bwilkerson FYI - this cropped up again today in flutter/flutter#102560. Some questions I'm still not sure of the answers to:

  • Whose responsibility will it be to create pngs from the source font/icon data?
    To show icons in VS Code we'd need PNGs, but the Flutter SDK doesn't include all the icons like that. Should the analysis server understand the source format used for Flutter and convert them, or should the flutter tool handle this?
    (I think this question was asked yesterday but I misunderstood it)
  • Should the solution (from the analysis servers point of view) work for non-Flutter icons
    For example if I'm writing a non-Flutter app that has icons (for ex. a standard Dart web app using Bootstrap Icons or similar), should there be a way to get the same icon previews in the IDE (without having to use Flutter classes)

I think it would be nice to avoid depending on Flutter, but I don't know if it's feasible (for example if it was the flutter tools job to extract(/download) PNGs, it would probably need to be done up-front and not on-the-fly, but it's not clear how that would work for icons in packages outside of the SDK (at least not without adding additional hooks that might make "flutter pub get" different to "pub get").

@bwilkerson
Copy link
Member

Should the solution (from the analysis servers point of view) work for non-Flutter icons

That would certainly be my preference. I don't understand enough about the situation yet to know whether doing so is practical.

@stevemessick
Copy link
Contributor

To make it work for non-standard icon packages we'd need some way to identify which packages were icon packages and which files in the package were font files. It would be nice if that could be added to the package itself, perhaps in the YAML file. In the IntelliJ plugin, I added a preference field where users could identify which packages were icon packages then used some heuristics to locate font files. All those I looked at included a class with a static field for each icon and I used additional heuristics to find those Dart files and use them to gen-up completion data.

The one thing Flutter and Java have that Dart might not have is a library for reading TTF/OTF font files.

@jacob314
Copy link
Member

The top priority is to support all Flutter icons from 1P and 3P Flutter packages.
I don't think supporting the non-Flutter use case is important because:

  1. It could be supported in the future by an analyzer plugin for the new non-Flutter UI framework. Think of what we are doing as embedding the Flutter Analyzer Plugin directly in the analyzer so it is ok to get a bit Flutter specific and generalize later.
  2. I can't find any prior art of a non-Flutter icon library that has 1 Dart const per icon or is otherwise well aligned with specifying icons from code rather than CSS in a way that overlaps with how Flutter does. I'd be open to adding annotations to package:meta if there were other somewhat popular packages the annotation could be added to.

It would also be nice leverage the analysis server / analyzer functionality for resolving icons to help DartDoc include previews of icons even if the author doesn't do anything special in the doc comments. That would also ensure that all are icon renders are consistent.

For example:
https://pub.dev/documentation/font_awesome_flutter/latest/font_awesome_flutter/FontAwesomeIcons-class.html
is missing icon previews in DartDoc even though it uses the same IconData class as the base Flutter icons.
https://api.flutter.dev/flutter/material/Icons-class.html

It would also make the doc comments for the Flutter icons cleaner if they didn't have to include HTML to render the icons. Right now that HTML is just noise if you jump to definition of an icon.

@bwilkerson
Copy link
Member

We might also be able to show previews in hovers.

@jacob314
Copy link
Member

jacob314 commented May 2, 2022

We might also be able to show previews in hovers.

Do you mean previews on hover when debugging or some other hover use case?

@bwilkerson
Copy link
Member

I was thinking about the editing experience in the IDE. I have no idea whether there would be enough information in the debugger to provide that kind of feedback.

@DanTup
Copy link
Collaborator Author

DanTup commented May 3, 2022

It would also make the doc comments for the Flutter icons cleaner if they didn't have to include HTML to render the icons.

My understanding is that this HTML is there to support the generated Dartdoc HTML on the web. Currently VS Code is regex-ing it out to replace with local icon previews but if we start appending/prepending icon previews into all IconData fields in the server, we might need to start regex-ing that out there to avoid it showing up "broken" in the editors (or, have Flutter/Dartdoc change how it works so it doesn't require that in the raw source).

The one thing Flutter and Java have that Dart might not have is a library for reading TTF/OTF font files.

Yeah, this might be reason to have the Flutter tool generate previews - although that might mean it would have to create them all up-front (eg. in the hook after running pub get) and put them somewhere. Right now there's code in flutter/tools_metadata that does this, but it's not exactly fast (so sitting waiting for it after adding an icon package or updating Flutter not be ideal).

@guidezpl
Copy link
Contributor

guidezpl commented Mar 16, 2023

I'm working on flutter/flutter#102560, which will be the replacement for Flutter's Icons, so-called Symbols, contained in a 3P package rather than in the framework. The current mechanism for generating files and storing them in the extension or https://github.com/flutter/tools_metadata is not desirable/scalable because the update pace for this package will be much quicker.

There is a new annotation, @staticIconProvider, which could be used to identify this style of class:

@staticIconProvider
class Symbols {
...
  /// <i class="material-icons-outlined md-36">10k</i> &#x2014; Material Symbol `10k`.
  /// https://fonts.google.com/icons?selected=Material+Symbols+Outlined:10k&icon.platform=flutter
  static const IconData ten_k = IconDataOutlined(0xe951);
...

My understanding is that this HTML is there to support the generated Dartdoc HTML on the web.

Correct. The CSS class material-icons-outlined turns the icon name into an icon. If I understand correctly, for hover cards we can't use the HTML+CSS solution that Dartdoc uses because, while VS Code allows it for their icons (octicons), they don't allow specifying additional CSS icon files? Is the only alternative generating .pngs? Are gutter icon previews equally limited to .pngs? I should emphasize this Symbols is backed by a variable font (fill, weight, grade, optical size), so resorting to static files to represent symbols is less than ideal.

@bwilkerson
Copy link
Member

There is a new annotation, @staticIconProvider, which could be used to identify this style of class:

Out of curiosity, where is the annotation defined?

@DanTup
Copy link
Collaborator Author

DanTup commented Mar 20, 2023

@guidezpl

@staticIconProvider
class Symbols {
...
  /// <i class="material-icons-outlined md-36">10k</i> &#x2014; Material Symbol `10k`.
  /// https://fonts.google.com/icons?selected=Material+Symbols+Outlined:10k&icon.platform=flutter
  static const IconData ten_k = IconDataOutlined(0xe951);
...

If the HTML here is specifically for Dartdoc (which I presume is the only client that has the correct CSS/fonts set up that it would render correctly), could this be extracted to something that the server/clients could understand more easily? For example, the embedded YT videos look like:

/// {@youtube 560 315 https://www.youtube.com/watch?v=Zbm3hjPjQMk}

Showing something similar (for ex. {@icon 10k}) instead of <i class="material-icons-outlined md-36">10k</i> &#x2014; Material Symbol '10k'. would be less noisy when reading the source code and may look less odd if it shows up in places like hovers. If we want to strip it out of hovers in the server, it would be much nicer to remove the annotation than have to keep a copy of the HTML Flutter is using (something that has changed at least once in the past).

If I understand correctly, for hover cards we can't use the HTML+CSS solution that Dartdoc uses because, while VS Code allows it for their icons (octicons), they don't allow specifying additional CSS icon files?

I thought this was the case, but while just looking around I came across this:

https://code.visualstudio.com/api/references/icons-in-labels#icon-contribution-point

This seems like it may be possible to provide a woff font and create your own. However, these needs to be contributed statically (that is, the woff file and a full list of mappings would need to be shipped in a/the VS Code extension and not versioned with the SDKs). This means a solution like that would also be VS Code-specific.

Is the only alternative generating .pngs? Are gutter icon previews equally limited to .pngs? I should emphasize this Symbols is backed by a variable font (fill, weight, grade, optical size), so resorting to static files to represent symbols is less than ideal.

We can use svg too, although I'm not sure if that's exactly what you were after.

@stevemessick
Copy link
Contributor

@guidezpl Please have a look at a spec for adding icon preview support to the analysis server.
https://docs.google.com/document/d/1h4vfkpamhJ-SN-jUESdmOp9MM0MjxMm00rpSeZ3gqZ0/edit?resourcekey=0-fBSEkPI1N0XcCr-S-qxAfw#

It mentions a C program to generate icons. I expect to have that up on GitHub within a few days.

@DanTup DanTup changed the title Provide a way for packages to provide icon previews to editors via the anlaysis server Support icon previews in editors via the analysis server Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants