[Feat] allow for multiple value column type #1666
Conversation
crabatel
commented
Dec 13, 2021
crabatel
commented
- add array type matching
- add array parsing
- add array ordinal domain generation
crabatel
force-pushed
the
add-multiple-value-column
branch
2 times, most recently
from
8deca7e
to
e42bd58
Compare
crabatel
force-pushed
the
add-multiple-value-column
branch
from
e42bd58
to
b3642fe
Compare
* add array type matching * add array parsing * add array ordinal domain generation
crabatel
force-pushed
the
add-multiple-value-column
branch
from
b3642fe
to
bfb12f1
Compare
|
Please |
| @@ -214,8 +214,6 @@ | |||
| "prettier": "1.19.1", | |||
| "progress-bar-webpack-plugin": "^2.1.0", | |||
| "puppeteer": "^2.1.1", | |||
| "react": "^16.8.4", | |||
There was a problem hiding this comment.
any reason why this is removed?
There was a problem hiding this comment.
Hello,
Actually I removed this because I thought having react as a dependency there was the cause of an error when trying to npm link the Kepler package (@see https://stackoverflow.com/questions/66975613/error-invalid-hook-call-hooks-can-only-be-called-inside-of-the-body-of-a-funct), but it appeared it wasn't the reason as we could not get rid of this error.
As we could not npm link we resolved to pushing the dist/ folder on our repo and updating our dependency to point to our repo, hence the presence of the dist/ folder.
Concerning this PR, we were not sure it could be accepted as we made the assumption that an array typed column in the CSV would be in a JSON format. This is the format we decided to use for our project, but it may not be the case for everyone, and maybe it should be discussed.
If the changes can be accepted, I'll remove the dist/ from this branch as well as the dependency change.
| return data => filter.value.includes(valueAccessor(data)); | ||
| return data => { | ||
| const value = valueAccessor(data); | ||
| if (Array.isArray(value)) { |
There was a problem hiding this comment.
To make this function more efficient, we should do the checking out side the data => boolean function
case FILTER_TYPES.multiSelect:
return feld.type === ALL_FILED_TYPES.array ? data => {
const value = valueAccessor(data);
return Array.isArray(value) && value.some(v => filter.value.includes(v));
} : data => filter.value.includes(valueAccessor(data));There was a problem hiding this comment.
Indeed it's better, I'll update this one.
…as-erasme/kepler.gl into add-multiple-value-column
.github/workflows/npmpublish.yml
Outdated
| @@ -1,8 +1,10 @@ | |||
| name: Node.js Package | |||
|
|
|||
| on: | |||
| release: | |||
| types: [created] | |||
| push: | |||
package.json
Outdated
| @@ -1,8 +1,8 @@ | |||
| { | |||
| "name": "kepler.gl", | |||
There was a problem hiding this comment.
please remove changes to package.json
src/constants/default-settings.js
Outdated
| @@ -46,7 +46,7 @@ import {TOOLTIP_FORMAT_TYPES} from './tooltip'; | |||
| import {LAYER_TYPES} from 'layers/types'; | |||
|
|
|||
| export const ACTION_PREFIX = '@@kepler.gl/'; | |||
| export const CLOUDFRONT = 'https://d1a3f4spazzrp4.cloudfront.net/kepler.gl'; | |||
| export const CLOUDFRONT = 'https://raw.githubusercontent.com/datatlas-erasme/front/master/src/src/static/'; | |||
src/layers/icon-layer/icon-layer.js
Outdated
| @@ -30,7 +30,7 @@ import {getTextOffsetByRadius, formatTextLabelData} from '../layer-text-label'; | |||
|
|
|||
| const brushingExtension = new BrushingExtension(); | |||
|
|
|||
| export const SVG_ICON_URL = `${CLOUDFRONT}/icons/svg-icons.json`; | |||
| export const SVG_ICON_URL = `${CLOUDFRONT}/icons.json`; | |||
> fix keplergl#1637 and supersede keplergl#1666 Allow to import features with multiple values properties and convert them to an `array` type. These fields can then be picked as filters or shown in the tooltip.
> fix keplergl#1637 and supersede keplergl#1666 Allow to import features with multiple values properties and convert them to an `array` type. These fields can then be picked as filters or shown in the tooltip.
> fix keplergl#1637 and supersede keplergl#1666 Allow to import features with multiple values properties and convert them to an `array` type. These fields can then be picked as filters or shown in the tooltip. Signed-off-by: lutangar <johan.dufour@gmail.com>
> fix keplergl#1637 and supersede keplergl#1666 Allow to import features with multiple values properties and convert them to an `array` type. These fields can then be picked as filters or shown in the tooltip. Signed-off-by: lutangar <johan.dufour@gmail.com>
