Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2408
@ns7381 I've started reviewing the PR. This is a great addition. Thank you
for taking the time to post it. Before we merge it in, I have a couple requests.
- The changes appear to cause the icons in the toolbar at the top to become
disable with a vanilla unsecured instance. I haven't been able to continue
verifying functionality as I'm blocked on this issue.
- The client-side code is structured in a modular format. Each module has
any dependent modules passed into it. Rather than directly referencing nf._ on
the global scope throughout the code base, can you please pass in a reference
to nf._ in each module that references it? See the top of any
nf-<module-name>.js for an example.
- The Messages-<county-code>.properties are under src/main/java. Can these
be moved to src/main/resources?
- Some of the keys in resources.js have generic names like Message11. Can
we use meaningful names throughout?
- I think we may need to introduce some defensive programming in some cases
now that the values which were previously string literals have been abstracted
and accessed via a lookup. Since there is no intention to support markup in the
values, we should not pass them into functions that support interpreting
markup. Instead that should only be set via `.text(...)` or escaped prior to
passing in to `.html(...)`, `.append(...)`, etc. You should see other instances
of this throughout the codebase, but I am happy to help identify specific cases
as the review progresses.
- What strategies are available for applying localization to messages
returned from the server? Should a follow-on JIRA be filed to help in this area?
Thanks!
---
