fix: remove dead links #450
Conversation
|
Thanks @landonreed ! Assigning to @binh-dam-ibigroup for final review of this small PR |
| </Checkbox> | ||
| </FormGroup> | ||
| </div> | ||
| ) | ||
| } | ||
| const mapStateToProps = (state, ownProps) => { | ||
| return { | ||
| termsOfStorageSet: state.otp.config.persistence?.terms_of_storage |
There was a problem hiding this comment.
Add a comment in example-config.yml to show this parameter works.
There was a problem hiding this comment.
Do we need a similar config variable for the terms of use?
There was a problem hiding this comment.
Have added an example in 6227886. I don't think a similar config item for terms of use makes sense since otherwise the terms of use checkbox would not refer to any terms of use to accept!
There was a problem hiding this comment.
Also, there is an existing component in ComponentContext in which the contents for the optional Terms of Storage is defined. Maybe you can use it in this PR to check for the existence of terms of storage instead of a new config property, or combine that in another PR. Thoughts?
There was a problem hiding this comment.
I gave this some thought, having an attribute on the "dummy" components shipped within otp-rr, and then if that attribute is missing showing the link. However, I found that all our configurations ship with the dummy components anyway, so that wouldn't work unless we changed all the configurations, which I wanted to avoid.
There was a problem hiding this comment.
Can't we just check for components.TermsOfStorage !== null?
There was a problem hiding this comment.
Given the rush I'm inclined to accept the approach, with issue #452 to deduplicate configuration.
|
Thanks Binh! All changes addressed |
|
🎉 This PR is included in version 3.4.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Removes help link that doesn't contain content and makes terms of storage more info link hidden by default unless explicitly enabled in the config.