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

Icon previews in hovers are broken for icons that start with numbers #3843

Closed
Rishaba-Jain opened this issue Feb 25, 2022 · 5 comments
Closed
Labels
in editor Relates to code editing or language features in flutter Relates to running Flutter apps is bug
Milestone

Comments

@Rishaba-Jain
Copy link

Describe the bug
I was working with icons and found that:

  1. Few icons are not previewed while hovering but are previewed near line numbers (checked the location where the icons are stored and it seems that there might be a mismatch in the mapping of names - screenshots attached)
  2. Few icons are not previewed while hovering nor near the line numbers (checked the location where the icons are stored and it seems that the icons are not there)

To Reproduce
Steps to reproduce the behavior:

  1. Please try to preview the icons in VSCode

Expected behavior
The icons must be previewed while hovering, near the line numbers.

Screenshots
Screenshot (1)
Screenshot (2)
Screenshot (3)
Screenshot (4)
Screenshot (5)
Screenshot (6)

Versions (please complete the following information):

  • VS Code Insiders version:
    Version: 1.65.0-insider (user setup)
    Commit: 3112460dc48ce7e557ea9feeaae04b912164b48b
    Date: 2022-02-24T05:15:20.989Z
    Electron: 16.0.9
    Chromium: 96.0.4664.174
    Node.js: 16.9.1
    V8: 9.6.180.23-electron.0
    OS: Windows_NT x64 10.0.19042
  • Dart extension version:
    Dart v3.35.20220222 Pre-Release
  • Flutter extension version:
    Flutter v3.35.20220201 Pre-Release
  • Dart/Flutter SDK version:
C:\Users\NEW>flutter --version
Flutter 2.10.2 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 097d3313d8 (6 days ago) • 2022-02-18 19:33:08 -0600
Engine • revision a83ed0e5e3
Tools • Dart 2.16.1 • DevTools 2.9.2

C:\Users\NEW>flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel stable, 2.10.2, on Microsoft Windows [Version 10.0.19042.1526], locale en-IN)
[√] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
[√] Chrome - develop for the web
[X] Visual Studio - develop for Windows
   X Visual Studio not installed; this is necessary for Windows development.
     Download at https://visualstudio.microsoft.com/downloads/.
     Please install the "Desktop development with C++" workload, including all of its default components
[√] Android Studio (version 2020.3)
[√] Connected device (3 available)
[√] HTTP Host Availability

! Doctor found issues in 1 category.

C:\Users\NEW>dart --version
Dart SDK version: 2.16.1 (stable) (Tue Feb 8 12:02:33 2022 +0100) on "windows_x64"

I am happy to contribute.

Please let me know if any further information is required.

Thanks for this wonderful plugin !!

@DanTup DanTup changed the title Icon failed to load in vscode Icon previews in hovers are broken for icons that start with numbers Feb 28, 2022
@DanTup DanTup added this to the v3.36.0 milestone Feb 28, 2022
@DanTup DanTup added in editor Relates to code editing or language features in flutter Relates to running Flutter apps labels Feb 28, 2022
@DanTup
Copy link
Member

DanTup commented Feb 28, 2022

Thanks for this wonderful plugin !!

Thanks, glad you like it!

I am happy to contribute.

Thanks for the offer, although this turned out to be a little fiddly. There are actually two issues:

  1. The icons we ship are missing the latest icons added to Flutter. We get these from https://github.com/flutter/tools_metadata so I have a PR out to update this (Re-generate from latest beta branch flutter/tools_metadata#30)
  2. Some icons that have CSS classnames that start with digits do not match the on-disk filenames (for example "3g" vs "threeg") and these need mapping (similar to https://github.com/flutter/flutter/blob/504e66920005937b6ffbc3ccd6b59d594b0e98c4/dev/tools/update_icons.dart#L44-L77). I'm working on handling this now.

@DanTup
Copy link
Member

DanTup commented Feb 28, 2022

  1. apparently there is another issue too, because even those that aren't affected by the issues above don't seem to work for me right now..

@DanTup
Copy link
Member

DanTup commented Feb 28, 2022

Nope, that one was just me testing it badly. Looks good now:

Screenshot 2022-02-28 at 19 05 55

Thanks!

@guidezpl
Copy link
Contributor

This won't work for a small set of icons which use a different for of rewrites (e.g. try_sms_star). I recommend using https://github.com/flutter/flutter/blob/504e66920005937b6ffbc3ccd6b59d594b0e98c4/dev/tools/update_icons.dart#L500, which handles both cases

@DanTup
Copy link
Member

DanTup commented Apr 6, 2022

Missed this before - I've opened #3913 about that. I can copy the exceptions from there, although they'll need to be maintained. I wonder if there's a better way that won't need so much maintenance (or if we could at least push some of this logic to https://github.com/flutter/tools_metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in flutter Relates to running Flutter apps is bug
Projects
None yet
Development

No branches or pull requests

3 participants