nvisionative / nvquickcomponents Goto Github PK

This is long overdue, but we need to get on the latest version of Stencil. We also need to clean up our package dependencies and devDependencies.

Add new tabs component

There is no CI build for pull requests, we should add something. I have setup VSTS builds with stenciljs if we want to go that route and I can share or we can use a script with AppVeyor. Either way we should set up something for CI.

I can help set this up

There is a lot of commit noise since the dist changes quite a bit with each build of stenciljs. In my own project I got around this by not including the dist folder in the repo and only use it when I package up the project for publishing to our local npm feed. We could try this or maybe we could setup a dev branch and master branch where the dev branch does not include the dist.

There is a bug in order versions of Stencil that give this result sometimes when using a Stencil component project inside another Stencil project:

After reading to old issues here and there it looks like this is resolved by upgrading the Stencil version on the imported project.

I will try to submit a PR for this tomorrow...

If you are using the autocomplete control with a remote endpoint to populate the results there can be a race condition if it takes your result some time to respond. See the example below with steps to reproduce

1. Start typing into auto complete (the first char or first 2 char may be an expensive query)
2. Continue typing until you get to 3 or 4+ chars where the query is much faster
3. Wait

Expected Result

• The autocomplete should show the correct records for the given input

Actual Result

• The autocomplete shows the correct records
• After a brief amount of time it flashes to incorrect records for a query that was waiting

Root Cause
There is a race condition happening with the getRemoteResults invocation which overrides results. We should add some code to the callback (maybe promise handling) to drop requests if they are not the latest.

The problem has to do with this invocation and callback

getRemoteResults(input, (remoteItemsSource) => {
    let results:HTMLElement[] = [];
    for (let index in remoteItemsSource) {
        let currentElement:HTMLElement = buildAutoCompleteItem(remoteItemsSource[index]);
        results.push(currentElement);
    }

    hydrateAutocomplete(results);
});

Publish new version 0.0.4 to npm.

Setup development branch. The master branch will still be the default branch for this repo, but we'll start processing PRs against development moving forward. See #3 for more information.

I created a private fork of this project for some close source projects to see how the components will handle in the real world. I am very happy with how things are working but we need to have a discussion around the shadow dom.

In my scenario I tested my new autocomplete control (i haven't made a PR yet) located at

https://github.com/HoeflingSoftware/nvQuickComponents/tree/autocomplete

The problem

I had issues with being able to apply css styles to the Shadow DOM vs no Shadow DOM. The challenge was I found myself starting to maintain 2 sets of styles a shadow DOM version and a polyfill version. I decided in my private repo that it would be best to not support the shadow DOM.

Browsers I tested

• Edge
• Firefox
• Chrome

Edge and Firefox looked perfect since they both used polyfill where chrome was the outlier that had issues.

When I submit the PR for the autocomplete I will not be using the Shadow DOM for it (unless otherwise directed during code review). I want to start the conversation about what our standard will be for the component library. There should be consistency across the components so they can be used and styled in similar fashions in the various projects.

See #11 for more information. With master being the default branch and development being the branch for new PRs, we need to create a CONTRIBUTING.md that makes it very clear the process moving forward.

The Autocomplete control displays extra results when there is a ',' in the result. We need to update our parsing logic to use the JSON object instead of splitting it by ','