UI: TTL picker cleanup #18114
UI: TTL picker cleanup #18114
Conversation
hashishaw
force-pushed
the
ui/ttl-picker-cleanup
branch
from
2678da1
to
88b6210
Compare
|
|
||
| module('Integration | Component | ttl picker', function (hooks) { | ||
| module('Integration | Component | ttl-picker', function (hooks) { |
There was a problem hiding this comment.
Contents of the test from ttl-picker2 replaced this test for a deleted component
| * automatically recalculates the time value when unit is updated unless time has been changed recently. | ||
| * Once all instances of TtlPicker are replaced with TtlPicker2, this component will be removed and | ||
| * TtlPicker2 will be renamed to TtlPicker. | ||
| * TtlPicker components are used to enable and select duration values such as TTL. This component renders a toggle by default and: |
There was a problem hiding this comment.
Contents of TtlPicker2 component replaced the unused TtlPicker component
| // initialValue: null, | ||
| // changeOnInit: false, | ||
| // hideToggle: false, |
There was a problem hiding this comment.
Do we want to keep these commented out values?
There was a problem hiding this comment.
I see what you mean with the template being less than ideal when incorporating the toggle but I think it's still worth it to get rid of the form component. Thanks for doing that! It's too bad that the
Togglecomponent has to wrap everything. I thought that it would be standalone similar to a checkbox.
Yeah, it doesn't have to wrap everything, but in order to get behavior where you can click on the label to toggle the content, it does have to wrap. A future update could be something where the parent can override the input ID so that we can match on a custom label, but I think that's out of scope for now.
| @@ -108,29 +113,29 @@ export default Component.extend({ | |||
| time = convertFromSeconds(seconds, unit); | |||
| } | |||
| } catch (e) { | |||
| // if parsing fails leave as default 30s | |||
| // if parsing fails leave it empty | |||
| return; | |||
There was a problem hiding this comment.
Do we want to avoid setting unit and enableTTl if the time parsing fails by returning here?
There was a problem hiding this comment.
I see what you mean with the template being less than ideal when incorporating the toggle but I think it's still worth it to get rid of the form component. Thanks for doing that! It's too bad that the Toggle component has to wrap everything. I thought that it would be standalone similar to a checkbox.
* UI: TTL picker cleanup (#18114) * Update config-pki.hbs Unable to make Pki changes because we don't have action handleCrlTtl * fix space * remove test --------- Co-authored-by: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
This PR does some cleanup with the TTL components. This also finishes up a long-lived project to replace all
<TtlPicker>components with<TtlPicker2>which was developed to replace it, and includes a toggle to enable.TtlPickerwithTtlPicker2TtlPicker,TtlForm,TtlPicker2) into one componentBecause there are so many moving pieces and similarly named files, I highly recommend reviewing this by stepping through the individual commits
TTL Picker with
@hideToggleTTL Picker
