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
Fix #15746: Don't set active topic to 'ALL' in contributor dashboard page. #15841
Conversation
Hi @sagangwee, can you complete the following:
|
Hi @DubeySandeep, could you please add the appropriate changelog label to this pull request? Thanks! |
@seanlip Could take a look at this? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM.
Just a question though. Where is the logic for "defaults to the first topic in the list"? Shouldn't we be initializing this.topicName somehow?
The HTML select list defaults to the first item in the topic list, but I've also explicitly set the active topic name to the first topic in the list. |
Sg, thanks! |
FYI Karma tests are failing. |
Hi @sagangwee, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks! |
@gp201 PTAL, thanks! |
Hi @sagangwee, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @sagangwee, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks! |
Hi @sagangwee, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks! |
Hi @sagangwee, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks! |
@sagangwee Any update here? |
@vojtechjelinek I've been trying to merge this PR for a few days now, but there are a couple of E2E flaky tests that are blocking. Same issue with #15851. Related to the |
…page. (#15841) * Don't set ALL topics * default to first topic * fix coverage * Remove fdescribe * fix lint
@sagangwee this PR appears to have introduced a flake into the frontend tests (see #16529). Can you please investigate? @gp201 has suggested a fix, so I think we can fix this forward instead of reverting |
@sagangwee have you looked into this yet? |
Overview
Essential Checklist
Proof that changes are correct
PR Pointers