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

feat(flutter): create package #173

Merged
merged 21 commits into from
Jul 5, 2022
Merged

Conversation

heavybeard
Copy link
Contributor

This branch is in progress by @Ciock and me, following #172 feature request.

I will delete WIP when it will be possible to make a review of this PR.

@lucaburgio lucaburgio requested a review from sammarks June 30, 2022 11:10
@heavybeard
Copy link
Contributor Author

heavybeard commented Jun 30, 2022

@lucaburgio, I have a couple of issues to discuss with you

1. Which are the rules to decide which icon name should be added to incompatibleNames?

In incompatibleNames variable there are a group of icon names because they start with numerical digits (to follow this guideline), but here there are other icons which start with numerical digits (all 3d-* ones).

So, why aren't these icons in the incompatibleNames variable? Is it an error or a choice?

2. Do you prefer to render with plain SVG?

Every icon file is an SVG, like this one.
Flutter doesn't support "natively" SVGs, but:

  • we can use a package like flutter_svg
    Than we can generate something like this with the build script
    import 'package:flutter/widgets.dart';
    import 'package:flutter_svg/flutter_svg.dart';
    
    class Iconoir extends StatelessWidget {
      const Iconoir({Key? key}) : super(key: key);
    
      @override
      Widget build(BuildContext context) => SvgPicture.asset('icons/iconoir.svg');
    }
  • we can try to paint with svg_to_paint
    Than we can generate something like this
    class Iconoir extends CustomPainter {
      final viewPortWidth = 24;
      final viewPortHeight = 24;
    
      String path0 =
          'M,23.4,12.3,C,23.4,11.5,23.299999999999997,10.700000000000001,23.2,9.9,H,12,V,14.5,H,18.4,C,18.099999999999998,16,17.299999999999997,17.3,15.999999999999998,18.1,V,21.1,H,19.799999999999997,C,22.1,19,23.4,15.9,23.4,12.3,Z';
      String path1 =
          'M,12,24,C,15.2,24,17.9,22.9,19.9,21.1,L,16.099999999999998,18.1,C,14.999999999999998,18.8,13.599999999999998,19.200000000000003,11.999999999999998,19.200000000000003,C,8.899999999999999,19.200000000000003,6.199999999999998,17.1,5.299999999999998,14.300000000000002,H,1.299999999999998,V,17.400000000000002,C,3.3,21.4,7.5,24,12,24,Z';
      String path2 =
          'M,5.3,14.3,C,4.8,12.8,4.8,11.200000000000001,5.3,9.700000000000001,V,6.6,H,1.2999999999999998,C,-0.40000000000000013,10,-0.40000000000000013,14,1.2999999999999998,17.299999999999997,L,5.3,14.3,Z';
      String path3 =
          'M,12,4.8,C,13.7,4.8,15.3,5.3999999999999995,16.6,6.6,L,16.6,6.6,L,20,3.2,C,17.8,1.2000000000000002,15,0.10000000000000009,12,0.10000000000000009,C,7.5,0,3.3,2.6,1.3,6.6,L,5.3,9.7,C,6.2,6.9,8.9,4.8,12,4.8,Z';
    
      @override
      void paint(Canvas canvas, Size size) {
        canvas.drawRect(Rect.fromLTRB(0, 0, size.width, size.height),
            Paint()..color = Color(0xFFffffff));
    
        canvas.drawPath(drawPath(path0, viewPortWidth, viewPortHeight, size),
            Paint()..color = Color(0xFF4285F4));
        canvas.drawPath(drawPath(path1, viewPortWidth, viewPortHeight, size),
            Paint()..color = Color(0xFF34A853));
        canvas.drawPath(drawPath(path2, viewPortWidth, viewPortHeight, size),
            Paint()..color = Color(0xFFFBBC04));
        canvas.drawPath(drawPath(path3, viewPortWidth, viewPortHeight, size),
            Paint()..color = Color(0xFFEA4335));
      }
    
      @override
      bool shouldRepaint(CustomPainter oldDelegate) => false;
    }

Copy link
Collaborator

@sammarks sammarks left a comment

Choose a reason for hiding this comment

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

Looks good with my limited knowledge of Dart! One question: does this support the automatic publishing that we already have in place for the React packages?

Constraints for proper versioning:

  1. The version should always match the version field of the main package.json
  2. If there is a publish process, it should automatically be done using GitHub Actions every time a new release is created inside the repository.

I'm not as familiar with the Dart ecosystem, but if importing packages is done via Git URLs, then I would assume that means there isn't any explicit publishing process other than adding the build artifacts to the repo?

@heavybeard
Copy link
Contributor Author

I would assume that means there isn't any explicit publishing process other than adding the build artifacts to the repo?

Dart uses pub.dev, It's like npm but for dart.

One question: does this support the automatic publishing that we already have in place for the React packages?

I'll add the necessary job needed to do that.

@heavybeard
Copy link
Contributor Author

@sammarks the PR is ready to be reviewed

  • I added the task to build the icons for flutter package
  • I added the task into prepublish to update the version of pubspec file programmatically
  • I added the step into release workflow to publish into pub.dev registry

Now you can follow these steps:

  1. Create the developer profile here → https://pub.dev/
  2. Create the verified publisher
  3. Follow this mini-guide https://github.com/marketplace/actions/publish-dart-flutter-package#credential
  4. Add the secret PUB_CREDENTIAL_JSON with the file content from step 3

@heavybeard heavybeard marked this pull request as ready for review July 1, 2022 11:32
@heavybeard heavybeard changed the title WIP feat(flutter): create package feat(flutter): create package Jul 1, 2022
@sammarks
Copy link
Collaborator

sammarks commented Jul 1, 2022

@lucaburgio can you create the developer profile here? It requires domain verification and I (understandably) don't have access to the Iconoir domain.

Once you do, can you invite me to it via my email address? And then I can handle the rest of the steps.

@sammarks
Copy link
Collaborator

sammarks commented Jul 1, 2022

@heavybeard to answer your questions:

1 seems like an error; though to my knowledge it has not posed a problem with React. If Dart doesn't support icons starting with numbers, feel free to create a dart-specific modification (but make sure not to change how the React icons are generated).

2 I will leave that up to you and your knowledge of Dart / Flutter to come up with the most acceptable approach. In NPM it's not a huge deal to have an additional dependency for rendering SVGs, but I'm not sure if that's the same with Dart. My knowledge of mobile development says it's best to limit dependencies if possible. But it looks like the second approach might be more limiting than the first?

@heavybeard
Copy link
Contributor Author

heavybeard commented Jul 2, 2022

@sammarks I pushed two more commits to include svg file's content inside the flutter widget in order to respect the react packages style

Flutter package way → https://github.com/lucaburgio/iconoir/blob/735643589ae5362e9a9a2a66989c614c6e08fbcd/packages/iconoir-flutter/lib/3_d_add_hole.dart#L13-L25

React packages way → https://github.com/lucaburgio/iconoir/blob/735643589ae5362e9a9a2a66989c614c6e08fbcd/packages/iconoir-react/src/3DAddHole.tsx#L7-L32

For #1 point, i follow the name of the 3D* icon classes prefixing them with Svg

Here is where I prefix with Svg the icon name when the icon file starts with a number → https://github.com/lucaburgio/iconoir/blob/735643589ae5362e9a9a2a66989c614c6e08fbcd/bin/build.js#L385-L390

And here is the result → https://github.com/lucaburgio/iconoir/blob/735643589ae5362e9a9a2a66989c614c6e08fbcd/packages/iconoir-flutter/lib/3_d_add_hole.dart#L4

In order to respect the react libraries class names → https://github.com/lucaburgio/iconoir/blob/735643589ae5362e9a9a2a66989c614c6e08fbcd/packages/iconoir-react/src/3DAddHole.tsx#L3

@heavybeard heavybeard requested a review from sammarks July 2, 2022 09:40
@lucaburgio
Copy link
Collaborator

@sammarks Invite sent ✅

@sammarks
Copy link
Collaborator

sammarks commented Jul 2, 2022

@lucaburgio can you give me access to edit secrets inside this repo so I can add the pub credentials? It's probably an admin role, so you can downgrade me back to maintainer after I've added the secret if you'd like.

Copy link
Collaborator

@sammarks sammarks left a comment

Choose a reason for hiding this comment

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

All looks good; I just need to configure the pub credential and then we should be good to go.

@lucaburgio
Copy link
Collaborator

@sammarks Configuration done ✅ Let me know if we can proceed 💪🏻

@sammarks sammarks merged commit c6d96ea into iconoir-icons:master Jul 5, 2022
@heavybeard heavybeard deleted the feat/flutter branch July 5, 2022 15:37
@sammarks
Copy link
Collaborator

sammarks commented Jul 5, 2022

Released! https://pub.dev/packages/iconoir_flutter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants