Skip to content
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

Select first available channel when accepting ownership change #3382

Merged

Conversation

Poslovitch
Copy link
Contributor

@Poslovitch Poslovitch commented Nov 29, 2020

Description

This is a draft as the implementation it introduces must be discussed.

Per request in #2240, I've slightly changed the select form in my-accept-ownership.component.html so that the first element of videoChannels is automatically selected. This, however, occurs regardless of the amount of channels an account has. This is the behavior I want to discuss with you. Should it remain as is, or should I refactor this PR to follow more closely the request of #2240?

Secondly, I had to change the disable condition of the "Accept" button. While the aforementioned change somehow worked fine if an account had multiple channels, the "Accept" button would remain greyed out if the account only had one channel.

Related issues

Resolves #2240

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 👍 yes, light tests as follows are enough
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

I don't know if tests should be added to the tests suite and/or modified.

Manual testing showed that the changes introduced by this PR are working as expected on my end.

Screenshots

These screenshots are taken just right after clicking on the "checkmark" button to open the popup.

When the account only has one channel
image

When the account has 2 or more channels
image

@Chocobozzz
Copy link
Owner

Chocobozzz commented Nov 30, 2020

I've slightly changed the select form in my-accept-ownership.component.html so that the first element of videoChannels is automatically selected. This, however, occurs regardless of the amount of channels an account has.

Auto select a channel is fine, we do this in many other places. The user can also change the video channel after the ownership change.

Regarding the code, use this.form.patchValue in the component instead of [selected]="i == 0" in the template. Then, I don't think we need to disable the accept button anymore with these changes

And removed the disabled state for the Accept button
@Poslovitch Poslovitch marked this pull request as ready for review December 1, 2020 13:07
@Chocobozzz Chocobozzz merged commit a3f1595 into Chocobozzz:develop Dec 1, 2020
@Chocobozzz
Copy link
Owner

Thanks @Poslovitch

@Poslovitch
Copy link
Contributor Author

Thanks for merging 🙂

@Poslovitch Poslovitch deleted the feature/2240-default-channel branch December 1, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default channel selection when owner change (if unique channel)
2 participants