Skip to content

Properties autocomplete#134

Open
aadsm wants to merge 5 commits into
montagestudio:masterfrom
aadsm:properties-autocomplete
Open

Properties autocomplete#134
aadsm wants to merge 5 commits into
montagestudio:masterfrom
aadsm:properties-autocomplete

Conversation

@aadsm

@aadsm aadsm commented Dec 20, 2013

Copy link
Copy Markdown
Contributor

⚠️ DO NOT MERGE, RFC only ⚠️

I'm highly skeptical about this implementation, here's the reasons:

  • The extension access the CodeEditor directly, the CodeEditorDocument should be the one defining which component to use to edit... The CodeEditor is the one that needs access to the projectController, however, waiting to load all the modules and properties only at that point in time might be too slow, it's better at extension activation.
  • Is there a better way to get the package require or loading a module?
  • At the moment I'm listening on projectController."moduleLibraryItemMap.keys()" to know when a new library item is available but this is ALL of them, it forces me do manually test if it's a dependency or not, I would prefer to use the projectController.dependencies but have to know how to listen on new library items there.
  • How to know that a blueprint has a new property defined?
  • Lots of 404's on the console due to nonexistent blueprints in some components, it would be nice to know if they exist first or not.

We weren't considering spaces between { and ".
Also added a sanity check to consumeString when readInitialQuote is true.
This was corrupting the state.
Also stop calling endProperty when it's not needed.
When multiple blocks are open in the same line we don't increase the
indentation size for the next line, otherwise we would increase
the indentation level by 2 with code like [{ or [[.
@mczepiel

Copy link
Copy Markdown
Contributor

I don't think there's anyway to avoid the 404ing for non-existent blueprints.

@mczepiel

Copy link
Copy Markdown
Contributor

The inspector is picking up on new properties as soon as they're added, so hmm I'll look into that.

@mczepiel

Copy link
Copy Markdown
Contributor

In answer to blueprint updates:

                    // we could create a binding to the propertyBlueprintGroups,
                    // but at the moment I'm not expecting the component blueprint
                    // to change at runtime
                    this.propertyGroupsController.content = value.propertyBlueprintGroups.map(function (groupName, index) {
                        return {
                            name: groupName,
                            properties: value.propertyBlueprintGroupForName(groupName),
                            open: index === 0
                        };
                    });

https://github.com/declarativ/palette/blob/master/ui/inspector/blueprint/blueprint-editor.reel/blueprint-editor.js#L52

EDIT: Within the associated template of the blueprinteditor, the properties are passed along to the propertyGroupEditor FWIW, so I imagine the properties here is the collection that changes
"properties": {"<-": "@propertyGroups.objectAtCurrentIteration.properties"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is the packageRequire you were asking about?

This is probably the best way unless we do it for you and make it available as a promise in the projectController itself.
FWIW we do the same thing in the reelDocument and maintain a private reference.

https://github.com/declarativ/filament/blob/master/core/reel-document.js#L2056

@aadsm

aadsm commented Jan 18, 2014

Copy link
Copy Markdown
Contributor Author

To implement this we were using the moduleId found in the library items. This information no longer exists since we generalized the library item.
A library item now has a moduleIds property with all the modules used but this is only populated when we when use the item in the library explorer. We opted for this to reduce the number of templates needed to be created.

Alternatives to this approach are:

  • Have an internal list of moduleIds maintained by using _findLibraryItemsForPackageUrl and by listening to new packages being installed and new files being created/deleted.
  • Have a new EnvironmentBridge function that returns all moduleIds that the applications depends on and maintained by watching file changes + package changes.
  • Have a principalModuleId in the LibraryItem.

Only the second option would allows us to have a comprehensive list of module ids.
For now I would recommend having a hardcoded list of most used packages: montage, digit, matte, glTF + listening on files created to know the modules of the application package.

@declarativ-bot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@Stuk

Stuk commented Feb 17, 2014

Copy link
Copy Markdown
Contributor

ok to test

@declarativ-bot

Copy link
Copy Markdown

Can one of the admins verify this patch? (Reply "ok to test")

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