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

feat(general): initialize default model #129

Merged
merged 21 commits into from
Mar 12, 2019
Merged

Conversation

sergiulucaci
Copy link
Contributor

This PR aims to resolve #124 ticket.

Default config initialization lives in a static method and the default properties are the minimum required for the UI to display and function properly.

Both config and controller methods are called createDefaultConfig() and default properties can be overwritten by passing the model as a parameter.

Note: I replaced the previous branch called: chore/elements-model-initialization with this one in order to have a cleaner commit history.

@sergiulucaci sergiulucaci requested a review from edeustace March 11, 2019 18:24
@sergiulucaci
Copy link
Contributor Author

An issue occurred related to extended-text-entry package. The UI panel doesn’t render due to a runtime error: regeneratorRuntime is not defined.

This error occurred during https://github.com/pie-framework/pie-elements/pull/121/files merge.

As a workaround I installed a new dev dependency @babel/plugin-transform-runtime: ^7.0.0 and use it into babel.config.js plugins as follows:

// babel.config.js 
…
   plugins: [
     '@babel/plugin-proposal-class-properties',
     '@babel/plugin-proposal-export-default-from',
     '@babel/plugin-proposal-export-namespace-from',
     '@babel/plugin-transform-runtime' <-- add this plugin
   ],
…

If you think this workaround is the right way to do it, I can submit a pull request with the fix.

@sergiulucaci
Copy link
Contributor Author

Another issue that I found is one related to the text-entry package.

By default, allowIntegersOnly value from the controller model equals to false, which means the Input from ~/pie-ui/packages/text-entry/src/input.jsx cannot render. This is because its inputComponent prop is null - see getFormatTag() method; while a string or a class/function is expected.

In this case I suggest to use inputComponent only if the returned value from getFormatTag() is different than null.

The changes will be made into ~/pie-ui/packages/text-entry/src/text-entry.jsx and will look like:

https://www.dropbox.com/s/l7gzt2gef0rzaqx/Screenshot%202019-03-12%2015.30.14.png?dl=0

Is this the right approach?

@edeustace
Copy link
Contributor

@sergiulucaci - re extended-text-entry build - yes add this to the root level babel config - thanks

@edeustace
Copy link
Contributor

@sergiulucaci re inputComp error ...

                inputComponent={
                  inputComponent === null || inputComponent === undefined
                    ? undefined
                    : inputComponent
                }

would work?

@sergiulucaci
Copy link
Contributor Author

@sergiulucaci re inputComp error ...

                inputComponent={
                  inputComponent === null || inputComponent === undefined
                    ? undefined
                    : inputComponent
                }

would work?

Yeah undefined does the trick.

Would you like to have the conditional under a constant similar to the screenshot above or would you like to have it into the inputComponent property?

@edeustace
Copy link
Contributor

i don't mind @sergiulucaci whatever you think reads best

@evaneus evaneus merged commit b62d058 into develop Mar 12, 2019
@evaneus evaneus deleted the feat/initialize-default-model branch March 12, 2019 19:47
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.

3 participants