Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Improve country dropdown UX and expose +prefix #833

Merged
merged 6 commits into from
Apr 26, 2017

Conversation

lukebarnard1
Copy link
Contributor

@lukebarnard1 lukebarnard1 commented Apr 25, 2017

A prefix is now exposed through a change to the API for onOptionChange. This now returns the entire country object which includes iso2, prefix etc.

This change adds the exposed prefix to phone number fields in the Registration and Login screens.

This is the second iteration on improving the login UI (element-hq/element-web#3524)

Also fixes not automatically selecting the first item in the dropdown: fixes element-hq/element-web#3686

traditional screenshot:
2017-04-25-115828_352x201_scrot

Luke Barnard added 3 commits April 25, 2017 11:21
A prefix is now exposed through a change to the API for onOptionChange. This now returns the entire country object which includes iso2, prefix etc.

This also shows the prefix in the Registration and Login screens as a prefix to the phone number field.
@@ -115,7 +115,7 @@ export default class Dropdown extends React.Component {

componentWillReceiveProps(nextProps) {
this._reindexChildren(nextProps.children);
const firstChild = React.Children.toArray(nextProps.children)[0];
const firstChild = nextProps.children[0];
Copy link
Member

Choose a reason for hiding this comment

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

I thought this was necessary? ie. children is essentially an opaque data structure and you needs to use React.children to interact with it?

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Apr 25, 2017

Choose a reason for hiding this comment

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

If you use React.children you get the full key of the child, something like .$GB. (I should've mentioned this TBF.)

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. I think there's still a problem though as it would be valid to pass only a single element to a dropdown, in which case this.props.children would not be an array?

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Apr 25, 2017

Choose a reason for hiding this comment

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

oh fair enough, I shall null guard it. Actually it would be an array with one item, but there might not be any children.

Copy link
Member

Choose a reason for hiding this comment

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

TBH, if it is still an array with one child that's probably fine - I'm not too fussed about supporting dropdowns with no options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have seen the dd get into that state first-hand so I think it'd be better to guard against it. There's nothing to highlight or to index, so it makes sense to return early.

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Apr 25, 2017
lukebarnard1 pushed a commit to element-hq/element-web that referenced this pull request Apr 25, 2017
@lukebarnard1 lukebarnard1 assigned dbkr and unassigned lukebarnard1 Apr 25, 2017
@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Apr 25, 2017
@lukebarnard1 lukebarnard1 assigned dbkr and unassigned lukebarnard1 Apr 25, 2017
@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Apr 25, 2017
@lukebarnard1 lukebarnard1 merged commit 4207bf3 into develop Apr 26, 2017
dbkr added a commit to element-hq/element-web that referenced this pull request Apr 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CountryDropdown does strange things when Enter is hit
2 participants