Use getInitialSelectedItem if this.state.selected is empty

[brunoscopelliti]

Proposed fix for #8913 .

Since the problem is that this.state.selected has lost its value, I try to use it when it is set, otherwise I use getInitialSelectedItem to get the initial selected value.

[alisterscott]

Thanks for your contribution @brunoscopelliti, it's very much appreciated.

I took a look at this and it works well and it seems to fix the issue I raised. I couldn't see any other issues introduced elsewhere in calypso, but since this is changing every select list in calypso I want to make sure that the code is correct, so I will ask someone else to do a code review, then I'm happy to ship it.

Before

After

[brunoscopelliti]

Thank you @alisterscott for the fast review.
If anybody else has some suggestion, it is more than welcome.

I noticed that the select-component dropdown is not covered by unit tests...
If you think this could be somehow useful, I will be happy to submit a new pull request with some tests in the next days.

Finally, I've a question: what do you use to record those screenshots?

[spen]

Blame shows that this hasn't been updated since the initial commit, @retrofox & @ehg look to have done work nearby so may be of some help.

I noticed that there are no unit tests for this component - perhaps we could support his PR with some coverage? I'd be happy to help out there!

Also +1-ing the gif tip!

[gwwar]

Code looks good here, but we can maybe decompose this a bit with a helper method:

getSelectedText() {
    const { options, selectedText } = this.props;
    const { selected } = this.state;
    if ( selectedText ) {
        return selectedText;
    }
    //return currently selected text
    const selectedValue = selected ? selected : this.getInitialSelectedItem( this.props );
    return result( find( options, { value: selectedValue } ) );
}

[brunoscopelliti]

Nice. This also pairs well with the idea of having unit tests...

I will update the pull request; by the way what is your workflow to update a pull request? Is it ok if I simply push a new commit on this branch?

[gwwar]

Is it ok if I simply push a new commit on this branch?

Yes please do. 😄 We also prefer to rebase branches with master (rather than merge) to keep history clean. We'll of course make sure you'll get credit for the fix, so let's try to squash the changes on this branch to a single commit, so @alisterscott can do a merge commit into master.

See also our contributing doc

[brunoscopelliti]

I've update the pull request.

Is it ok that now other commits are bundled up in this pull request?

I've not great experience with rebase in Git, and I'm not sure I've followed the process correctly. @gwwar Could you please review the steps, and correct me if I did something wrong/not necessary?

I followed the following steps:

On my master branch

git pull Automattic master
git push

On the fix/issue-8913 branch

git rebase master

At this point the status was somehow not consistent:

#  Your branch and 'origin/fix/issue-8913' have diverged,
#  and have 10 and 1 different commits each, respectively.

I thought I've to do a rebase also with origin/fix/issue-8913. So still from the fix/issue-8913 branch

git rebase origin/fix/issue-8913

This restored the state, and I was able to proceed; I edited the file, commit, squash, and push.

ps. I don't care too much about the attribution of the commit (I'm here primarily to learn), if I screwed up the pull request seriously feel free to discard it, and restore the fix in a different branch.

[gwwar]

You can think of rebase as picking out your updates, and replaying them on a fresh master. Your branch will diverge from origin if someone else has pushed to origin master. (See visual here: https://onlywei.github.io/explain-git-with-d3/ )

At this point

#  Your branch and 'origin/fix/issue-8913' have diverged,
#  and have 10 and 1 different commits each, respectively.

Your branch was fine, you just needed a git push --force to push changes to the origin branch. (Note that this is ok to do since you're the only one working in the branch).

To clean this up please rebase again with master, and do a force push to your origin branch. Let us know if you still need help and I can do this manually.

[gwwar]

Almost, 😄 I think 68fbbcb snuck in. Perhaps make sure your local master is up to date.

[brunoscopelliti]

@gwwar Thank you very much for the explanation! I think now it should be ok.

[gwwar]

👍 Left a suggestion, but this looks good to me.

[gwwar]

Finally, I've a question: what do you use to record those screenshots?

I'm not sure what workflow @alisterscott uses, but I personally do a screen recording with QuickTime, then convert the movie into a gif using gify

[alisterscott]

Finally, I've a question: what do you use to record those screenshots?

I used LiceCap which is free/open source and records animated GIFs directly from your screen - link

[gwwar]

@alisterscott did you want to do a last test and merge this in? Let's make sure this goes in as a merge:

[alisterscott]

Retested and LGTM - 🚢 this now