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.

Sign up for GitHub

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

fix mapping in codebook so the newValue can accept any data type #496

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

fix mapping in codebook so the newValue can accept any data type #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebb6c3e
fix mapping so it can accept any type of value
dayesouza Jan 24, 2023
e51bf1b
apply version and lint fixes
dayesouza Jan 24, 2023
7ff30e4
fix lint and update function call
dayesouza Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .yarn/versions/1bde106c.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
releases:
"@datashaper/app-framework": patch
"@datashaper/react": patch
"@datashaper/stories": patch
"@datashaper/webapp": patch
7 changes: 2 additions & 5 deletions javascript/react/src/components/Codebook/MappingFields/MappingFields.hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,16 @@
* Licensed under the MIT license. See LICENSE file in the project.
*/

import { DataType } from '@datashaper/schema'
import { useCallback } from 'react'

export function useHandleAddButtonClick(
onChange?: (mapping: Record<any, any>) => void,
values?: Record<any, any>,
dataType?: DataType,
): () => void {
return useCallback(() => {
const newValue = dataType === DataType.Number ? 0 : ''
onChange?.({
...values,
' ': newValue,
' ': '',
})
}, [onChange, dataType, values])
}, [onChange, values])
}
8 changes: 2 additions & 6 deletions javascript/react/src/components/Codebook/MappingFields/MappingFields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,11 @@ export const MappingFields: React.FC<CodebookMappingFieldProps> = memo(
const handleKeyChange = useHandleKeyChange(values, onUpdateMapping)
const handleValueChange = useHandleValueChange(
values,
field.type,
undefined,
onUpdateMapping,
)
const handleDelete = useHandleDelete(values, onUpdateMapping)
const handleButtonClick = useHandleAddButtonClick(
onUpdateMapping,
values,
field.type,
)
const handleButtonClick = useHandleAddButtonClick(onUpdateMapping, values)

const columnPairs = useMappingPairs(
field.mapping ?? {},
Expand Down
3 changes: 2 additions & 1 deletion javascript/react/src/hooks/controls/mapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* Licensed under the MIT license. See LICENSE file in the project.
*/

import { type Value, DataType } from '@datashaper/schema'
import type { Value } from '@datashaper/schema'
import { DataType } from '@datashaper/schema'
import { useCallback } from 'react'

export function useHandleKeyChange(
Expand Down
19 changes: 7 additions & 12 deletions javascript/react/src/hooks/controls/useMappingPairs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
ITextFieldStyleProps,
ITextFieldStyles,
} from '@fluentui/react'
import { Icon, IconButton } from '@fluentui/react'
import { Icon, IconButton, TextField } from '@fluentui/react'
import { memo, useMemo } from 'react'

import { DataTypeField } from '../../components/verbs/forms/shared/DataTypeField.js'
Expand Down Expand Up @@ -76,10 +76,10 @@ const ColumnPair: React.FC<{
disabled,
}) {
// the old value will always come off the map as a string key
// coerce it to the column type for proper comparison
const [o, q] = valuePair
// coerce the key to the column type for proper comparison
// the newValue will always be a string since it can be of any type
const [o, newValue] = valuePair
let keyValue = o !== ' ' ? coerce(o, dataType) : o
const propertyValue = coerce(q, dataType)

if (dataType === DataType.Boolean) {
o === 'false' ? (keyValue = false) : (keyValue = true)
Expand Down Expand Up @@ -107,15 +107,10 @@ const ColumnPair: React.FC<{
styles={{ root: { marginLeft: 4, marginRight: 4 } }}
/>

<DataTypeField
<TextField
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we losing the custom type entry stuff here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tor the value received yes, so the new mapped value can be anything. Shouldn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another thought here: we have talked about adding a secondary codebook data type to indicate the new mapping “to” (maybe called “mappedType”). In that case we can just use a Freeform text box and just parse the string into that new type. It’ll be a less “dynamic” interface, but much simpler to deal with

Copy link
Contributor

Choose a reason for hiding this comment

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

Tor the value received yes, so the new mapped value can be anything. Shouldn't it?

In the DataTypeField, the idea was supposed to be that if the type is a Boolean, it’s just true/false, if it’s a number it’s a spinner, etc

Copy link
Contributor

Choose a reason for hiding this comment

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

All we really need to do is make sure the data type for all mappings ends up the same since we can’t have mixed types in a column. The lowest common denominator is a string

Copy link
Contributor

Choose a reason for hiding this comment

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

Anything more dynamic is user convenience (and probably the only dynamic input that is really useful is the calendar picker for dates)

placeholder={'New Value'}
dataType={dataType}
keyValue={keyValue}
value={propertyValue}
onKeyChange={onValueChange}
onValueChange={onValueChange}
isKey={false}
dropdownStyles={dropdownStyles}
value={newValue}
onChange={(_: Value, val?: string) => onValueChange(keyValue, val)}
disabled={disabled}
/>

Expand Down