clouway / ui-components

A set of UI components used for development
0 stars 1 forks source link

ui-components: integration of modal in native #17

Closed gadjevb closed 6 years ago

gadjevb commented 6 years ago

Integration of a simple modal component in the native part, so it can be later reused in the different apps.

Fixes #16 modaldemo

gadjevb commented 6 years ago

Requested changes are made.

p-nedelchev commented 6 years ago

Can you consider modal to be opened with a flag like RN Modal there are some cases when you can't use trigger or refs?

mgenov commented 6 years ago

Do we have cases with such usages ?

p-nedelchev commented 6 years ago

Actually yes at least when I was migrating activate assets modal there should be opened when a specific error occurred so I can't use a trigger or can't call ref from the reducer. At least that's the way I could solve this so I change the modal in my PR (please have a look https://github.com/clouway/telcong.wss/pull/539/files#diff-674edb0335cb7e016ff6c534ecfa05bdR58)

martinmilev commented 6 years ago
return React.cloneElement(child, {
        onPress: () => {
          if (typeof onPressCall === 'function') {
            if (onPressCall.then !== undefined) {
              onPressCall()
                .then(result => {
                  this.props.onShowDialog()
                  return Promise.resolve(result)
                })
                .catch(error => {
                  return Promise.reject(error)
                })
            } else {
              onPressCall()
              this.props.onShowDialog()
            }
          } else {
            this.props.onShowDialog()
          }
        }
      })
gadjevb commented 6 years ago

PR is rebased and integrated into storybook. Should opening with flag be implemented?

martinmilev commented 6 years ago

Write README.md with description.

gadjevb commented 6 years ago

Flag for opening the modal is included.

gadjevb commented 6 years ago

I'm not able to hide the StatusBar, I think it's a bug with react native. Here's some more info: https://github.com/facebook/react-native/issues/7474 - Issue about the problem. https://github.com/facebook/react-native/pull/18004 - PR for the issue opened 3 days ago.

mgenov commented 6 years ago

Ok, as it's just for the example we can live with that for now.

gadjevb commented 6 years ago

Ready for a review!

gadjevb commented 6 years ago

All requested changes are fixed.

gadjevb commented 6 years ago

@mgenov requested changes are fixed.