create progress bar component #560

2019-05-29T09:01:01+02:00

akinwale commented

2019-05-29 09:01:01 +02:00

(Migrated from github.com)

Replaces use of the platform-dependent ProgressBarAndroid component.

Replaces use of the platform-dependent `ProgressBarAndroid` component.

neb-b (Migrated from github.com) reviewed 2019-05-29 09:01:01 +02:00

kauffj (Migrated from github.com) approved these changes 2019-05-29 16:52:55 +02:00

kauffj (Migrated from github.com) left a comment

Basically looks good. Out of curiosity, what was wrong with ProgressBarAndroid from ReactNative?

Basically looks good. Out of curiosity, what was wrong with `ProgressBarAndroid` from ReactNative?

app/src/component/progressBar/view.js

					
				@ -0,0 +16,4 @@

				    progress: function(props, propName, componentName) {

				      const value = parseInt(props[propName], 10);

				      if (isNaN(value) || props[propName] < minProgress || props[propName] > maxProgress) {

				        return new Error('progress should be between 0 and 100');

kauffj (Migrated from github.com) commented

2019-05-29 16:51:15 +02:00

Should this return an error or throw one?

app/src/styles/fileList.js

					
				@ -74,4 +74,1 @@

				    height: 3,

				    flex: 1,

				    flexDirection: 'row'

				  },

kauffj (Migrated from github.com) commented

2019-05-29 16:52:10 +02:00

Outside of this PR, but if 720 is a special value that appears in multiple places it should be a constant.

akinwale (Migrated from github.com) reviewed 2019-05-29 16:58:34 +02:00

app/src/component/progressBar/view.js

					
				@ -0,0 +16,4 @@

				    progress: function(props, propName, componentName) {

				      const value = parseInt(props[propName], 10);

				      if (isNaN(value) || props[propName] < minProgress || props[propName] > maxProgress) {

				        return new Error('progress should be between 0 and 100');

akinwale (Migrated from github.com) commented

2019-05-29 16:58:34 +02:00

According to https://reactjs.org/docs/typechecking-with-proptypes.html, return.

akinwale commented

2019-05-29 17:01:58 +02:00

(Migrated from github.com)

Basically looks good. Out of curiosity, what was wrong with ProgressBarAndroid from ReactNative?

It is Android specific, so it wouldn't work on iOS. I'd have to use the ProgressViewIOS too, which would require adding additional platform checks in the code. Made sense to just use a single component without having to perform these checks.

> Basically looks good. Out of curiosity, what was wrong with `ProgressBarAndroid` from ReactNative? It is Android specific, so it wouldn't work on iOS. I'd have to use the `ProgressViewIOS` too, which would require adding additional platform checks in the code. Made sense to just use a single component without having to perform these checks.

Sign in to join this conversation.

No reviewers

neb-b

kauffj