feat: subway nav additions

helenosheaa commented 2 years ago

Closes https://github.com/influxdata/clockface/issues/753 Helps with https://github.com/influxdata/data-acquisition/issues/240

Changes

I've added two optional props: settingUpHeader which defaults to "Setting Up", if a prop is not handed in. And showCheckmark which defaults to true unless it's handed in.

Both these changes are things which are needed only in the subscriptions edit view. Below see video of the changes to the edit view, the create view still looks the same and has no additional props needing to be handed in.

No checkmarks on progress through the single page edit view & subscription name in place of setting up text.

Screenshots

https://user-images.githubusercontent.com/38860767/163233086-3dc86632-5ac6-4ba6-bb74-d9893f014f3a.mov

@hoorayimhelping has agreed to publish once this has been approved and I'll swap it out in the UI.

Checklist

Check all that apply

[ ] Updated documentation to reflect changes
[x] Added entry to top of Changelog with link to PR (not issue)
[x] Tests pass
[ ] Peer reviewed and approved
[x] Signed CLA (if not already signed)

helenosheaa commented 2 years ago

thanks for the speedy review, I've updated the version number, changelog and made your suggested change.

brandenTenbrink commented 2 years ago

Everything looks fine to me, my only suggestion is would we want to use a defaultProps to set "Setting up" instead of having it done inline code. Makes it more readable imo.

@helenosheaa @hoorayimhelping

helenosheaa commented 2 years ago

@brandenTenbrink, do we have a practice of adding static default props like that in clockface? I'm a fan of an inline operator in something simple like this, but not against changing it.

@hoorayimhelping thoughts?

hoorayimhelping commented 2 years ago

@helenosheaa @brandenTenbrink I prefer to use default props as well. I find it generally conceptually simpler to comprhened when the default value of a property is set in a single place. If a prop is used in multiple places, the default will have to be set multiple times.

But at the same time, we don't use defaultProps anywhere in Clockface, so I don't think we should block this PR on it, unless Branden feels very strongly about it. If it's more of Branden's preferred way of doing things and not a hard requirement, we can work together to set a standard for it going forward, and replace inline uses.

influxdata / clockface