-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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(material-experimental/mdc-radio): de-duplicate test harness logic #21532
Conversation
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
I'm seeing the following error from this (and some of the other PRs to share harness code):
The code that causes it looks like this: import {MatRadioButtonHarness, MatRadioGroupHarness} from '@angular/material/mdc-radio/testing';
...
let radioGroupHarness: MatRadioGroupHarness[];
let radioButtonRelations: MatRadioButtonHarness[];
...
radioGroupHarness =
await harnessLoader.getAllHarnesses(MatRadioGroupHarness);
radioButtonRelations = await radioGroupHarness[0].getRadioButtons(); |
It's odd that our own build didn't complain about it. Maybe we can flip on a compiler flag to make it behave closer to g3? Also is it possible that they're importing both the non-MDC and MDC harnesses and they're using one to type the variable and the other one to look up the elements? |
That's what I assumed was happening at first, but no - they're not importing anything from radio/testing and the variables are explicitly typed with the class imported from mdc-radio/testing like I showed above. |
Maybe it has something to do with the return new HarnessPredicate<NonMdcMatRadioButtonHarness>(MatRadioButtonHarness, options) |
I see, I managed to reproduce it locally too. It's only the second call that is causing the compilation error. I think that it'll have to use the same approach where we have a base class for the harnesses. I'll rework it and let you know when it's done. |
Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.
962399c
to
53e6bde
Compare
Reworked the PR to avoid the compilation error. |
Passes TGP - will be merged on the next round |
…angular#21532) Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.
…angular#21532) Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.
…angular#21532) Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.
…angular#21532) Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Changes the MDC-based radio harnesses to extend the base ones directly since the logic is identical, aside from some selectors.