design-system-components icon indicating copy to clipboard operation
design-system-components copied to clipboard

Feature angular

Open shakilsiraj opened this issue 5 years ago • 20 comments

Angular 5 implementation of the uikit project. Has the core and accordion features implemented so far.

Comes with the following feature:

  • Based on the uikit CSS framework.
  • Component implementation based on Angular Component Development Kit (@angular/cdk).
  • Animation implementation using Angular Animation library. Does not directly use @gov.au/animate but replicates the functionality using @angular/animations.
  • Unit testing with angular test bed
  • A demo project to see components in action.

More component implementation in Angular to come I hope ...

shakilsiraj avatar Jul 02 '18 00:07 shakilsiraj

Thanks for the PR @shakilsiraj, we really appreciate all the work that's gone into this!

And thanks for the review @dominikwilkowski :)

Our lead developer is back from holiday in a couple days and we'll have a more detailed look at the code then. This will be a great opportunity to discuss how we might support another JS framework in the design system. I know there are a few agencies that are currently using Angular so this is a good conversation to have.

pattyde avatar Jul 02 '18 01:07 pattyde

There may be enough people in government using Angular to maintain this. It would be great having the per-package snippets Angular requires, the same way React is handled.

:+1:

mgdhs avatar Jul 02 '18 04:07 mgdhs

This is a great conversation and I thank you @shakilsiraj for starting it :)

The way we have used this mono repo is by adding each entry point for each framework to each component rather than creating new ones. That way we can reuse css etc. The way you added this in one monolithic chuck breaks this unfortunately

I thought about doing the same but it would have doubled up on the packages for little benefit. Let's not even mention that there will be an angular cli build project in each of the libraries. So I have taken angular material approach - one project with multiple exportable library packages. Have a look at https://github.com/angular/material2.

This is a good approach but completely at odds with what we got here with pancake. Pancake takes each entry point and extracts each resource to a folder o your choosing. Adding the angular pipeline for the site tests is fine too and will only add some time to bootstrapping the monorepo. Each component is installable via npm and by that built in isolation. This way users can install each component they need and nothing else saving bandwidth and making ci runs much faster.

This library does does not create alternative CSS for the other libraries, rather it requires those CSS files to be included manually (https://github.com/shakilsiraj/uikit/blob/feature-angular/packages/ngx/src/sass/_module.scss) into Angular projects that use this library. So, I don't think it's an issue.

This is done by pancake completely automatically if you use it. So this double up diverges from the pancake integration.

Introducing several new ways of building this is probably not the best strategy if you do that only in one single component. If we want to move to karma/jasmine we should do that everywhere not in one component and end up with several testing frameworks that all do the same. I for one would suggest staying with Jest and adding cypress for e2e tests to cover everything.

If you are going to support Angular cli based projects then there is no other way but to go with Karma/Jasmine. A single project with this exception(not using Jest and use the appropriate libraries that are required) will keep it under control. Jest is a good framework no doubt but it's an added library that you will need to support on top of what Angular is providing. In Angular world, you can try to not use Karma/Jasmin, but you will be doing extra work and will need to maintain that work for yourself.

That's unfortunate... good to know.

typescript, while awesome and amazing, is also something we should have a discussion about. I like typed languages but should probably pick one that does not diverge from the standard so we're not locked into an ecosystem (typescript has a commenting syntax that would help here)

Angular does not support vanilla JS (directives, enums, proper OOP, etc. are missing there). So, there is not alternative.

That would be a blocker for me here. We should not block us into a non-standard corner like we did with coffescript and many others before. This creates tech debt in the future. I know it's the "in" lib right now but I caution about jumping on a boat here that won't let you off easily.

Also in general a PR with 83 files changed is probably a bit much to be merged unless clearly communicated before hand with the team. I do however totally appreciate the effort you put into this.

Thanks for that and sorry for not communicating before hand. Most of those files in this PR would be standard from angular CLI when you start a project. The bit that you should be focusing on is https://github.com/shakilsiraj/uikit/tree/feature-angular/packages/ngx/src/app/modules. Next PRs from anyone should be small.

Point taken. Fair enough.

Happy to discuss.

Thank you for kicking this off. Looking forward to talking through some of those things and unblocking them. I am keenly aware that there is little docs about how pancake works here and how we set new modules up.

dominikwilkowski avatar Jul 03 '18 01:07 dominikwilkowski

Hey @alex-page, thanks for your comment.

I had a close look at the other tools (furnace, pancake) that you have been talking about and now I understand how they hang together. Some cool stuff you guys have there :).

Unfortunately, with this setup, I can't see how you can integrate Angular without bloating the existing packages. Angular is a beast by itself and requires an entire CLI project to be created just for a small library. If I go with the Angular pancake module, I will need to create a separate CLI project like the ngx one in each of the modules. That will slow down installation and build speed significantly especially for those who are not working with Angular.

A better alternative perhaps is to start fresh with a separate Angular project and pull in the required UIKIT projects via npm dependencies. That would be cleaner and easy to maintain. This can be hosted on a separate repo on its own. I'm sure there are enough people to support the project.

Happy to discuss.

shakilsiraj avatar Jul 27 '18 05:07 shakilsiraj

Thanks for the positive vibes and understanding @shakilsiraj definitely keen to talk more about this. Feel free to send an email to [email protected] and we can organise a meeting in person or on the phone!

I'm about to head off for the day, so i'll try give you a better reply about the approach for angular components on Monday 👍

alex-page avatar Jul 27 '18 06:07 alex-page

@shakilsiraj Ok I have some time to reply now. Again this pull request is awesome and really appreciate all of your hard work.

If I go with the Angular pancake module, I will need to create a separate CLI project like the ngx one in each of the modules. That will slow down installation and build speed significantly especially for those who are not working with Angular.

So this shouldn't be as big of an issue as you think. We have ways to enable and disable the build pipelines in pancake. This means that a user who wants angular would be able to get angular components in a folder. Users who do not want angular will not be affected.

The components themselves and the libraries that make them work are actually separated. When we deliver a react component we deliver pure component code but no library to make it work. When a user downloads a component and wants to use the react pipeline in pancake, they would literally only get the component file ( accordion.js ). We rely on the user to install react, npm, node and import the component in to their project. The same would also happen for angular we would expect the user to install angular or set up a cli that can then ingest the angular components.

I'll see if I can do a similar thing with a simple angular component and share it back to you and see what you think.

One thing we do is create a test page for each component. This would require angular to be added as a developer dependency. This would not be included as a dependency when installing the components.

A better alternative perhaps is to start fresh with a separate Angular project and pull in the required UIKIT projects via npm dependencies. That would be cleaner and easy to maintain. This can be hosted on a separate repo on its own. I'm sure there are enough people to support the project.

I think this is a valid solution, however I would love angular to be in this library and supported by the community. Splitting the projects for now may mean that you can work faster and are not blocked by us. If you do choose to go down this path I would love to one day soon sit down and discuss your approach and learnings and see how/if we can merge the two together.

alex-page avatar Jul 30 '18 05:07 alex-page

Hey @alex-page, I did send you an email with my contact number to [email protected]. Just wondering if you have received it or not? Love to have a chat. Thanks.

shakilsiraj avatar Aug 03 '18 04:08 shakilsiraj

@shakilsiraj received cheers 👍

alex-page avatar Aug 06 '18 04:08 alex-page

Hey @shakilsiraj thanks for the chat today. For everyone else's benefit here are somethings that came out of it.

We need the components to work with whatever bundler the user is using. This means it should work with rollup, angular cli or webpack. This also means that when we build an angular component it needs to be a pure component ( one javascript file that gets imported ) that should work in any build pipeline.

For our test pages we will need to use a build pipeline to actually render the angular component to the page, something like angular-cli. This would only be included as a developer dependency for this task and the user would be able to choose other implementations to suit themselves.

Things we and @shakilsiraj will try and do this sprint:

  • Set up the helper.js file to do an angular pipeline to build from src -> lib
  • Build a simple component direction-links/src/js/angular.js in angular
  • Render this to a test page
  • Publish a next version of the component and test how it integrates with an external angular pipeline
  • Start writing technical articles explaining how to get started with the design system using HTML, react and angular

alex-page avatar Aug 07 '18 05:08 alex-page

Hi guys,

It was nice chatting with you yesterday.

I think I have a good idea now in terms of bundling Angular components with rollup and test with jest. But I would like to get clarification re the build process that will generate the angular package to be consumed by apps. When I looked into @gov.au/accordian, I can see that the main entry in package.json points to 'lib/js/react.es5.js' even though the actual lib directory has the JQuery version. I guess for JQuery, you can just refer this file from html script tag, but it will be better to point to the angular lib file so the compiler can do a proper AOT compilation in the consuming app. How would you manage other library files (for example, lib/js/ngx.es5.js) to be exposed as the main entry point? Is there going to be another package created like @gov.au/accordian-ngx by the furnace tool?

The other option is to use a proper Angular library generator like this one - https://www.npmjs.com/package/ng-packagr. It will create an entire dist directory with the required bundles (es5, es2015, esm5, package.json, etc.) that can be used for publishing to npm directly. Can furnace create a separate angular package from this dist directory?

Happy to discuss. Thanks.

shakilsiraj avatar Aug 08 '18 01:08 shakilsiraj

Hey @shakilsiraj you make a really good point!

I think for now we can just import it with the path to the file, as we can only have one default file for now 😢 . Happy to look into other solutions to make this easier in the future. 👍

alex-page avatar Aug 08 '18 02:08 alex-page

I think(????) I have managed to get a good crack at it. Check out the the commit 0eeb2b4 in regards to #471 and #472.

Gist:

  1. There is an angular implementation under src/ts directory.
  2. There is a barebone angular demo app for the component under test/angular director.
  3. Jest with angular preset has been used to run unit tests - see setup-jest.ts and jest definition in package.json as well as test:unit-test target
  4. build:angular would create the distribution lib at lib/js/angular.es2015.js.
  5. test:angular would compile the demo site under test/angular directory.

I'm guessing you guys would be able to work out #470 and #473 from the fork? I might have missed something here, so would appreciate you guys having a go at it. Please let me know if you have any questions.

shakilsiraj avatar Aug 14 '18 09:08 shakilsiraj

Thanks @shakilsiraj I'm going to look at your work over the next few days and see what we can do to get this integrated correctly!

alex-page avatar Aug 30 '18 02:08 alex-page

:+1:

shakilsiraj avatar Aug 30 '18 05:08 shakilsiraj

Hi @shakilsiraj I think I have an idea about how this might be able to work from a simple approach. Let me know what you think.

The uikit could provide a template html and css file for ingestion by angular. This means that the html file could look like:

<div class="au-callout {{ classnames }}">{{ text }}</div>

Together they could be ingested like:

import { Component } from '@angular/core';

@Component({
  selector: 'au-callout',
  templateUrl: './au-callout.html',
  styleUrls: ['./au-callout.css']
})

export class AUcallout {
  text: 'a string or a variable',
  classnames: ' a string or variable',
}

Then our test pages could be a simple angular-cli repository ( however with a lot of the fuzz removed ).

The main thing I am not sure about is dynamic components, we can provide the spot for the class to go and click handlers however we wouldn't be writing the functionality in this case. Which may be beneficial of developers who want to control the state in other ways.

Let me know if you have any suggestions or ideas, i'm trying to keep the changes as minimal as possible so we can get this work in easily.

alex-page avatar Aug 31 '18 03:08 alex-page

Hey @alex-page,

I think I have done something very similar. Have a look at the following:

The angular module: https://github.com/shakilsiraj/uikit/tree/feature-angular/packages/direction-links/src/ts

The test application ( a barebone version of an ng cli project: https://github.com/shakilsiraj/uikit/tree/feature-angular/packages/direction-links/tests/angular/src

And the changes to package.json(https://github.com/shakilsiraj/uikit/blob/feature-angular/packages/direction-links/package.json) along with the new build targets: test:angular and build:angular.

What do you think?

shakilsiraj avatar Sep 02 '18 00:09 shakilsiraj

@shakilsiraj Yeah exactly something like this, I might try making a new branch with your code and see if I can make it a little bit smaller.

Thanks again for all your work and feedback!

alex-page avatar Sep 02 '18 23:09 alex-page

@sukhrajghuman are you able to remove me as reviewer 🙏

alex-page avatar Jul 31 '19 19:07 alex-page

@alex-page doesn't look like I can... :(

image

@shakilsiraj might be able to

sukhrajghuman avatar Jul 31 '19 23:07 sukhrajghuman

@alex-page you're not a reviewer (or what GitHub calls it assignee) anymore and it only shows your last review on the top. You can just mute the issues for yourself. :)

Screen Shot 2019-08-01 at 9 43 03 am

dominikwilkowski avatar Jul 31 '19 23:07 dominikwilkowski