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

Generalisation of lowercase etc enforcements on inputs #69

Merged
merged 9 commits into from
Jun 2, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
30 changes: 15 additions & 15 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
{
"dist/rifm.umd.js": {
"bundled": 7480,
"minified": 1724,
"gzipped": 858
"bundled": 8089,
"minified": 2023,
"gzipped": 987
},
"dist/rifm.min.js": {
"bundled": 7266,
"minified": 1583,
"gzipped": 780
"bundled": 7216,
"minified": 1570,
"gzipped": 779
},
"dist/rifm.cjs.js": {
"bundled": 7008,
"minified": 1650,
"gzipped": 831
"bundled": 7794,
"minified": 2077,
"gzipped": 975
},
"dist/rifm.esm.js": {
"bundled": 6937,
"minified": 1587,
"gzipped": 787,
"bundled": 7723,
"minified": 2014,
"gzipped": 933,
"treeshaked": {
"rollup": {
"code": 14,
Expand All @@ -29,9 +29,9 @@
}
},
"dist/rifm.esm.production.js": {
"bundled": 6651,
"minified": 1372,
"gzipped": 664,
"bundled": 6597,
"minified": 1359,
"gzipped": 663,
"treeshaked": {
"rollup": {
"code": 14,
Expand Down
26 changes: 23 additions & 3 deletions pages/case-enforcement/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ const Example = () /*:React.Node*/ => {
<div>Lower case</div>
<Rifm
accept={/./g}
format={v => v.toLowerCase()}
format={v => v}
replace={v => v.toLowerCase()}
value={lowercase}
onChange={setLowercase}
>
Expand All @@ -28,7 +29,8 @@ const Example = () /*:React.Node*/ => {
<div>Upper case</div>
<Rifm
accept={/./g}
format={v => v.toUpperCase()}
format={v => v}
replace={v => v.toUpperCase()}
value={uppercase}
onChange={setUppercase}
>
Expand All @@ -40,7 +42,8 @@ const Example = () /*:React.Node*/ => {
<div>Capital first letter</div>
<Rifm
accept={/./g}
format={v => v.slice(0, 1).toUpperCase() + v.slice(1).toLowerCase()}
format={v => v}
replace={v => v.slice(0, 1).toUpperCase() + v.slice(1).toLowerCase()}
value={capitalized}
onChange={setCapitalized}
>
Expand All @@ -59,6 +62,23 @@ const Example = () /*:React.Node*/ => {
{renderInput}
</Rifm>
</div>

<div>
<div>Leave a comment about Rifm</div>
<Rifm
accept={/./g}
format={v => v}
replace={v =>
'Rifm is the best mask and formatting library. I love it! '
.repeat(20)
.slice(0, v.length)
}
value={capitalized}
onChange={setCapitalized}
>
{renderInput}
</Rifm>
</div>
</Grid>
);
};
Expand Down
45 changes: 36 additions & 9 deletions src/Rifm.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type Props = {|
onChange: string => void,
format: (str: string) => string,
mask?: boolean,
replace?: string => string,
accept?: RegExp,
children: ({
value: string,
Expand Down Expand Up @@ -46,6 +47,19 @@ export const Rifm = (props: Props) => {
userValue === props.format(eventValue), // isNoOperation
];

if (process.env.NODE_ENV !== 'production') {
const formattedEventValue = props.format(eventValue);
if (
eventValue !== formattedEventValue &&
eventValue.toLowerCase() === formattedEventValue.toLowerCase()
) {
console.warn(`
Case enforcement does not work with format.
Please use replace={value => value.toLowerCase()} instead
istarkov marked this conversation as resolved.
Show resolved Hide resolved
`);
}
}

// The main trick is to update underlying input with non formatted value (= eventValue)
// that allows us to calculate right cursor position after formatting (see getCursorPosition)
// then we format new value and call props.onChange with masked/formatted value
Expand Down Expand Up @@ -78,7 +92,7 @@ export const Rifm = (props: Props) => {

const valueBeforeSelectionStart = clean(
eventValue.substr(0, input.selectionStart)
).toLowerCase();
);

// trying to find cursor position in formatted value having knowledge about valueBeforeSelectionStart
// This works because we assume that format doesn't change the order of accepted symbols.
Expand All @@ -87,20 +101,16 @@ export const Rifm = (props: Props) => {
// inputValue = 1'23'|4 so valueBeforeSelectionStart = 123 and formatted value = 1'2'3'4
// calling getCursorPosition("1'2'3'4") will give us position after 3, 1'2'3|'4
// so for formatting just this function to determine cursor position after formatting is enough
// with masking we need to do some additional checks see `replace` below
// with masking we need to do some additional checks see `mask` below
const getCursorPosition = val => {
let start = 0;
let cleanPos = 0;

for (let i = 0; i !== valueBeforeSelectionStart.length; ++i) {
let newPos =
val.toLowerCase().indexOf(valueBeforeSelectionStart[i], start) + 1;
let newPos = val.indexOf(valueBeforeSelectionStart[i], start) + 1;

let newCleanPos =
clean(val.toLowerCase()).indexOf(
valueBeforeSelectionStart[i],
cleanPos
) + 1;
clean(val).indexOf(valueBeforeSelectionStart[i], cleanPos) + 1;

// this skips position change if accepted symbols order was broken
// For example fixes edge case with fixed point numbers:
Expand Down Expand Up @@ -137,7 +147,24 @@ export const Rifm = (props: Props) => {
// if nothing changed for formatted value, just refresh so userValue will be used at render
refresh();
} else {
props.onChange(formattedValue);
if (process.env.NODE_ENV !== 'production') {
const replaceValue =
props.replace != null
? props.replace(formattedValue)
: formattedValue;

if (replaceValue.length !== formattedValue.length) {
console.warn('replace operation to work, must preserve length');
istarkov marked this conversation as resolved.
Show resolved Hide resolved
}

props.onChange(replaceValue);
} else {
props.onChange(
props.replace != null
? props.replace(formattedValue)
: formattedValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Put replaceValue into upper scope and reuse it here please. It will be inlined by minifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, 10 bytes gzipped lost

Copy link
Contributor

Choose a reason for hiding this comment

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

Readability of this fragment is bad. Only warning condition should be wrapped with NODE_ENV

);
}
}

return () => {
Expand Down
12 changes: 12 additions & 0 deletions tests/RifmFormat.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,15 @@ test('format works even if state is not updated on equal vals', async () => {
exec({ type: 'PUT_SYMBOL', payload: 'x' }).toMatchInlineSnapshot(`"123|’456"`);
exec({ type: 'PUT_SYMBOL', payload: 'x' }).toMatchInlineSnapshot(`"123|’456"`);
});

it('format can work with case changes', () => {
const exec = createExec({
format: v => v,
replace: v => v.toLowerCase(),
accept: /.+/g,
});

exec({ type: 'PUT_SYMBOL', payload: 'HELLO WORLD' }).toMatchInlineSnapshot(`"hello world|"`);
exec({ type: 'MOVE_CARET', payload: -5 }).toMatchInlineSnapshot(`"hello |world"`);
exec({ type: 'PUT_SYMBOL', payload: 'BeAuTiFuL ' }).toMatchInlineSnapshot(`"hello beautiful |world"`);
});
2 changes: 2 additions & 0 deletions tests/utils/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Props = {|
// replace?: string => boolean,
mask?: boolean,
format: (str: string) => string,
replace?: (str: string) => string,
maskFn?: string => boolean,
|};

Expand All @@ -33,6 +34,7 @@ export const createExec = (props: Props) => {
onChange={input.set}
accept={props.accept}
format={props.format}
replace={props.replace}
mask={
props.mask != null
? props.mask
Expand Down