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

[TS migration] Migrate 'getPermittedDecimalSeparator' lib to TypeScript #30160

Merged
merged 5 commits into from
Nov 7, 2023
Merged

[TS migration] Migrate 'getPermittedDecimalSeparator' lib to TypeScript #30160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
433a54b
Rename getPermittedDecimalSeparator
BartoszGrajdek Oct 23, 2023
1760004
[TS migration] Migrate 'getPermittedDecimalSeparator' lib to TypeScript
BartoszGrajdek Oct 23, 2023
5d8dfec
QFix
BartoszGrajdek Oct 24, 2023
cf37fbf
Resolve merge conflicts
BartoszGrajdek Oct 29, 2023
3640c82
Prettier fix
BartoszGrajdek Oct 29, 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
3 changes: 0 additions & 3 deletions src/libs/getPermittedDecimalSeparator/index.ios.js
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions src/libs/getPermittedDecimalSeparator/index.ios.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// On iOS keyboard can only have one symbol at a time (either dot or comma) so we accept both
// Details: https://expensify.slack.com/archives/C01GTK53T8Q/p1658936908481629
import GetPermittedDecimalSeparator from './types';

const getPermittedDecimalSeparator: GetPermittedDecimalSeparator = () => '.,';

export default getPermittedDecimalSeparator;
11 changes: 0 additions & 11 deletions src/libs/getPermittedDecimalSeparator/index.js
View file
Open in desktop

This file was deleted.

14 changes: 14 additions & 0 deletions src/libs/getPermittedDecimalSeparator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import getOperatingSystem from '@libs/getOperatingSystem';
import CONST from '@src/CONST';
import getPermittedDecimalSeparatorIOS from './index.ios';
import GetPermittedDecimalSeparator from './types';

const getPermittedDecimalSeparator: GetPermittedDecimalSeparator = (localizedSeparator) => {
if (getOperatingSystem() === CONST.OS.IOS) {
return getPermittedDecimalSeparatorIOS(localizedSeparator);
Copy link
Contributor

Choose a reason for hiding this comment

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

@BartoszGrajdek Is there a reason to be passing localizedSeparator here now? I think is not necessary as it is importing getPermittedDecimalSeparatorIOS from './index.ios' and this param is not used there, so we should keep the previous behavior

Copy link
Contributor Author

@BartoszGrajdek BartoszGrajdek Oct 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't want to change the functionality of these functions, keeping them consistent in types whether it's ios or other plaforms. That's why I've added it as a mandatory parameter, even though it's not being used on ios. If I have changed it to localizedSeparator?: string I would need to provide a default value to getPermittedDecimalSeparator because I don't want to return anything else other than string

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @BartoszGrajdek, the param isn't used in one implementation, but from the usage it does make sense to make it required. Wdyt @fabioh8010?

Copy link
Contributor

Choose a reason for hiding this comment

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

@BartoszGrajdek Thanks for explanation, makes sense to me!

}

return localizedSeparator;
};

export default getPermittedDecimalSeparator;
3 changes: 3 additions & 0 deletions src/libs/getPermittedDecimalSeparator/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type GetPermittedDecimalSeparator = (localizedSeparator: string) => string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type GetPermittedDecimalSeparator = (localizedSeparator: string) => string;
type GetPermittedDecimalSeparator = (localizedSeparator?: string) => string;

Should be optional


export default GetPermittedDecimalSeparator;
Loading