feat(material/stepper): allow for orientation to be changed dynamically #22139
Conversation
crisbeto
added
P2
google-cla
bot
added
the
cla: yes
|
Caretaker note: let me know if this causes any breaking changes and I can make some adjustments. I've tried to keep everything backwards-compatible, but there are a lot of changes so something might have slipped through. |
crisbeto
force-pushed
the
7700/stepper-orientation-again
branch
2 times, most recently
from
cd5896f
to
4d466e5
Compare
| @@ -34,8 +34,6 @@ import {MatStepContent} from './step-content'; | |||
| ], | |||
| exports: [ | |||
| MatCommonModule, | |||
| MatHorizontalStepper, | |||
| MatVerticalStepper, | |||
There was a problem hiding this comment.
Technically this change shouldn't be necessary, but for some reason keeping the old steppers in the exports causes a null pointer exception in the ViewEngine compiler.
src/material/stepper/stepper.ts
Outdated
| * @docs-private | ||
| */ | ||
| @Directive() | ||
| abstract class _MatProxyStepperBase { |
There was a problem hiding this comment.
I don't totally follow why you need _MatProxyStepperBase- why can't the shim horizontal/vertical steppers just extend MatStepper? Is it to avoid repeating the host bindings for ViewEngine compatibility?
There was a problem hiding this comment.
I wanted to avoid having to repeat the metadata. Also I believe that the template and styles would be inlined multiple times if I repeated them in the metadata of multiple components. That being said, I'm not 100% sure that it's actually possible for people get a hold of a MatVerticalStepper or MatHorizontalStepper instance after these changes, but they might still have references to the types. Alternatively, I can turn them into interfaces with the same public API, but it may result in a breaking change if they're referenced as a value, rather than a type.
There was a problem hiding this comment.
What if we just had MatStepper and we did e.g.
/** @deprecated */
export class MatVerticalStepper extends MatStepper { }
@Component({
...,
providers: [{provide: MatVerticalStepper, useExisting: MatStepper}],
})
export class MatStepper {
}The extends here is purely to fake MatVerticalStepper being the right type while still keeping it a real class that people can import.
There was a problem hiding this comment.
I already had the providers, but another problem is that MatVerticalStepper has to be declared after MatStepper, because it extends it, but it also can't be provided in the providers of MatStepper since it hasn't been declared yet. Using forwardRef seems to crash the compiler when used inside provide.
I've tweaked the approach to make it more manageable by having _MatProxyStepperBase extend CdkStepper and declare the Material-specific APIs on top of it. I've also added a couple of tests to make sure that the deprecated symbols work.
src/material/stepper/stepper.ts
Outdated
| * @docs-private | ||
| */ | ||
| @Directive() | ||
| abstract class _MatProxyStepperBase { |
There was a problem hiding this comment.
I already had the providers, but another problem is that MatVerticalStepper has to be declared after MatStepper, because it extends it, but it also can't be provided in the providers of MatStepper since it hasn't been declared yet. Using forwardRef seems to crash the compiler when used inside provide.
I've tweaked the approach to make it more manageable by having _MatProxyStepperBase extend CdkStepper and declare the Material-specific APIs on top of it. I've also added a couple of tests to make sure that the deprecated symbols work.
crisbeto
force-pushed
the
7700/stepper-orientation-again
branch
from
4d466e5
to
62d3ec6
Compare
Combines `mat-vertical-stepper` and `mat-horizontal-stepper` into a single `mat-stepper` class in order to allow for the orientation to be changed dynamically. Also deprecates `MatVerticalStepper` and `MatHorizontalStepper`. This is a reimplementation of angular#9173, however this time I took a different approach which should make it easier to maintain and eventually remove the two separate steppers. It should result in a smaller bundle as well. The main differences are: 1. Rather than have 3 components (`MatStepper`, `MatVerticalStepper` and `MatHorizontalStepper`), these changes combine everything into `MatStepper` while `MatVerticalStepper` and `MatHorizontalStepper` are only used as injection tokens for backwards compatibility. The `selector` and `exportAs` of `MatStepper` is changed to match the two individual steppers and the orientation is inferred from the tag name. This will make it much easier to remove the deprecated directives. Furthermore, it should result in a smaller bundle since the template and styles only need to be inlined in one place. 2. `MatVerticalStepper` and `MatHorizontalStepper` are turned into very basic directives that have the same public API as `MatStepper` and they proxy everything to it. This is primarily so that if somebody managed to get a hold of a `MatVerticalStepper` or `MatHorizontalStepper` instance, or they used the old classes to type their own code, it wouldn't result in a breaking change. Relates to angular#7700.
crisbeto
force-pushed
the
7700/stepper-orientation-again
branch
from
62d3ec6
to
718ca86
Compare
jelbourn
added
the
merge ready
|
|
|
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. |
Combines
mat-vertical-stepperandmat-horizontal-stepperinto a singlemat-stepperclass in order to allow for the orientation to be changed dynamically. Also deprecatesMatVerticalStepperandMatHorizontalStepper.This is a reimplementation of #9173, however this time I took a different approach which should make it easier to maintain and eventually remove the two separate steppers. It should result in a smaller bundle as well. The main differences are:
MatStepper,MatVerticalStepperandMatHorizontalStepper), these changes combine everything intoMatStepperwhileMatVerticalStepperandMatHorizontalStepperare only used as injection tokens for backwards compatibility. TheselectorandexportAsofMatStepperis changed to match the two individual steppers and the orientation is inferred from the tag name. This will make it much easier to remove the deprecated directives. Furthermore, it should result in a smaller bundle since the template and styles only need to be inlined in one place.MatVerticalStepperandMatHorizontalStepperare turned into very basic directives that have the same public API asMatStepperand they proxy everything to it. This is primarily so that if somebody managed to get a hold of aMatVerticalStepperorMatHorizontalStepperinstance, or they used the old classes to type their own code, it wouldn't result in a breaking change.Relates to #7700.