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

Added dynamic category capability #254

Merged
merged 4 commits into from
Jul 2, 2014
Merged

Added dynamic category capability #254

merged 4 commits into from
Jul 2, 2014

Conversation

TJKoury
Copy link
Contributor

@TJKoury TJKoury commented Jun 30, 2014

From this discussion, node-red now has dynamic category ingest ability. Categories can still be sub-indexed by using additional dashes, but all will be rolled under the a master category, as before. This version keeps the core modules in the correct order, as well.

@knolleary
Copy link
Member

Looking good.

Only issue is the palette had sub categories - xyz_input, xyz_output and xyz_function - to help with the ordering within a category - that appears to have been lost.

@TJKoury
Copy link
Contributor Author

TJKoury commented Jun 30, 2014

That was an intentional design choice; there's no limit to how deep
someone could nest subcategories, and nothing stopping them from using a
previously ingested node name as a subcategory name.

I guess we could limit it to one sub-level but then you would have to
discern intent with no a priori, like the difference between the core
input-http node and a new user node with a category input-http (full path
something like input-http-newnode). In this case, would your previous node
become a sub category unto itself, with two subnodes?

If you want to go the sub-category route I'll have to add in a name checker
algorithm that would alert the user that their node is trying to use a
subcategory name that collides with the canonical name of a pre-ingested
node; this would also mean that we'll have to re-write the ingester code to
look in certain paths first (like the core). All of that seems like a lot
to maintain rather than just sticking with one level of category.

tjk
On Jun 30, 2014 11:43 AM, "knolleary" notifications@github.com wrote:

Looking good.

Only issue is the palette had sub categories - xyz_input, xyz_output and
xyz_function - to help with the ordering within a category - that appears
to have been lost.

Reply to this email directly or view it on GitHub
#254 (comment).

@knolleary
Copy link
Member

I only meant I want you to restore the sub categories we already have defined for the existing categories.

How want want user defined categories to be structured is part of the bigger rethink of the palette. For this pr, we just want to enable single level user defined categories.

@TJKoury
Copy link
Contributor Author

TJKoury commented Jun 30, 2014

Should users be allowed to add to the core categories?
On Jun 30, 2014 1:03 PM, "knolleary" notifications@github.com wrote:

I only meant I want you to restore the sub categories we already have
defined for the existing categories.

How want want user defined categories to be structured is part of the
bigger rethink of the palette. For this pr, we just want to enable single
level user defined categories.

Reply to this email directly or view it on GitHub
#254 (comment).

@knolleary
Copy link
Member

Yes.

For the existing categories we want exactly the behaviour we have today (the exact set and structure of categories) that can be used by any node.

@TJKoury
Copy link
Contributor Author

TJKoury commented Jul 1, 2014

Added back the sub-categories, for every category it will create the three standard sub-category containers.

@TJKoury
Copy link
Contributor Author

TJKoury commented Jul 1, 2014

Fixed.

On Jun 30, 2014 1:47 PM, "knolleary" notifications@github.com wrote:

Yes.

For the existing categories we want exactly the behaviour we have today
(the exact set and structure of categories) that can be used by any node.

Reply to this email directly or view it on GitHub
#254 (comment).


RED.palette = function() {

var exclusion = ['config','unknown'];
Copy link
Member

Choose a reason for hiding this comment

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

This should be:

['config','deprecated']

@knolleary
Copy link
Member

Tested locally - all looks good but for one final thing - the exclusion list needs 'deprecated' adding.

@TJKoury
Copy link
Contributor Author

TJKoury commented Jul 2, 2014

Done!

On Wed, Jul 2, 2014 at 4:46 PM, knolleary notifications@github.com wrote:

Tested locally - all looks good but for one final thing - the exclusion
list needs 'deprecated' adding.

Reply to this email directly or view it on GitHub
#254 (comment).

knolleary added a commit that referenced this pull request Jul 2, 2014
Added dynamic category capability
@knolleary knolleary merged commit 6e956ef into node-red:master Jul 2, 2014
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.

2 participants