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

NIFI-1850 - Initial Commit for JSON-to-JSON Schema Converter Editor #424

Closed
wants to merge 3 commits into from

Conversation

YolandaMDavis
Copy link
Contributor

This is an initial commit for review of the TransformJson Advanced Editor. Please see https://issues.apache.org/jira/browse/NIFI-1850 for details.

@YolandaMDavis YolandaMDavis force-pushed the NIFI-1850-0.x branch 3 times, most recently from b51434f to bcc5586 Compare May 9, 2016 21:01

'use strict';

angular.module('standardUI').factory('ProcessorService', function ProcessorService($http) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Angular invokes certain functions (like service factories and controllers) via the injector. You need to annotate these functions so that the injector knows what services to inject into the function. Here you have chosen to do so implicitly... If you plan to minify your code and have used implicit annotations then your service names will get renamed and your angular app will be broken. To allow the minifiers to rename the function parameters and still be able to inject the right services, the function needs to be annotated with the $inject property. You can read more about it here: https://docs.angularjs.org/guide/di

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scottyaslan thanks you'll see the annotations on the next update. Also ensured height of code editor is responsive.

@mcgilman
Copy link
Contributor

@YolandaMDavis I just read through your PR and I wonder if it makes sense to simply reference JS libraries from nifi-web-ui instead of bundling them again. I realize that we're a long way off from establishing a set of components/widgets that are meant for other UI extension points to consume but this would help reduce the limit of the size of UI. Specifically, I was thinking about any angular (for 1.x) and codemirror.

If you're storing anything in your processor's annotationData your processor can implement Searchable to allow users to find your processor based on it.

@YolandaMDavis
Copy link
Contributor Author

@mcgilman had same conversation with @scottyaslan last week concerning referencing js in nifi-web-ui. My main concern was versioning e.g. changes happening at that level that a custom ui wouldn't be ready for. I can switch it without issue just think that in the future will have to manage that somehow.

@mcgilman
Copy link
Contributor

@YolandaMDavis Yep. We're definitely on the same page here. However, we currently have not formally established any APIs/components/widgets for UI extensions to consume. Because of this I think we're ok. In this case referencing stuff bundled in nifi-web-ui is fair game with the understanding that they may change with any release. During that release would we would also need to update this custom UI appropriately.

Going forward, we should establish APIs/components/widgets for custom UIs to consume with guarantees around their APIs. However, we need to be mindful that when we do this we won't be able to freely break those APIs. I would prefer to decouple these from any particular framework/versions so we're not 'stuck' with anything. We can obviously discuss this in more details as custom UIs become more prominent.

@YolandaMDavis
Copy link
Contributor Author

@mcgilman no problem. will refer to just codemirror in nifi-web-ui for 0.x and both angular and codemiror in 1.0 (since angular is only available in that module in 1.0)

@YolandaMDavis YolandaMDavis force-pushed the NIFI-1850-0.x branch 5 times, most recently from 590e02f to 0bffce7 Compare May 20, 2016 21:32
@mcgilman
Copy link
Contributor

mcgilman commented May 30, 2016

@YolandaMDavis The licensing updates look good.

However, now I'm having some issues saving the JSON specification in the Custom UI while running in a clustered instance. When I click save, the value for the specification is reset to the previous value and I do not get any message indicating an issue.

@mcgilman
Copy link
Contributor

mcgilman commented Jun 1, 2016

Functionality after the most recent update looks good. I've made a few minor stylistic changes and updated method visibility here [1]. Will merge to 0.x baseline. Thanks!

[1] mcgilman@502ac66

asfgit pushed a commit that referenced this pull request Jun 1, 2016
- Initial Commit for JSON-to-JSON Schema Converter Editor
- file re-org and licensing
- fixes for cluster (properties weren't updating)
- Minor style updates.
- Removing unecessary method from ComponentFacade.
- This closes #424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants