-
Notifications
You must be signed in to change notification settings - Fork 255
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
Paul/bitwave csv #4397
Paul/bitwave csv #4397
Conversation
6cb626f
to
99d76f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The export logic is simple enough, but there is a problem with the toggle switches on Android.
isExportQbo: !state.isExportQbo, | ||
isExportBitwave: !state.isExportBitwave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Android toggles are messed up. We need to do the following:
handleQboToggle = () => {
if (android) {
this.setState({ isExportQbo: true, isExportCsv: false, isExportBitwave: false })
} else {
this.setState(state => ({ isExportQbo: !state.isExportQbo }))
}
}
And likewise for the other 2 handlers. Then in the renderAndroidSwitches
, we use these 3 handlers, instead of the generic handleAndroidToggle
.
This is because Android can only choose one option at a time. With two options, it doesn't really matter which is which, but with 3 options, it suddenly matters which switch you choose, so we need separate handlers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. will do.
f224d01
to
da8bd7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's closer, and we could merge it if we really had to, but I see a way the user could crash it.
if (Platform.OS === 'android' && !this.state.isExportQbo) { | ||
this.setState({ isExportCsv: false, isExportBitwave: false }) | ||
} | ||
|
||
this.setState(state => ({ isExportQbo: !state.isExportQbo })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you press the radio button once on Android, it will select the option. If the press the ratio button again, it will de-select the option and leave nothing chosen. Then, when you hit submit, you will get a crash, because files[0]
is undefined.
The way I wrote the handler in my original review comment would have avoided this possibility, by ensuring exactly one item can be selected on Android.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't submit if nothing is selected. The Next button is hidden. This is also true on iOS. I did test this case. But it's fair to say the user shouldn't be allowed to select nothing. Admittedly I was trying to keep the logic similar to iOS wrt allowing a deselection
da8bd7d
to
4089e34
Compare
4089e34
to
fd61651
Compare
/rebase |
fd61651
to
9428775
Compare
9428775
to
0a32ce9
Compare
CHANGELOG
added: Export Bitwave format CSV
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: