Skip to content

LG-5468: Update IAL2 full-page ID error to show three troubleshooting options#5729

Merged
aduth merged 3 commits intomainfrom
aduth-lg-5468-doc-auth-tips
Dec 22, 2021
Merged

LG-5468: Update IAL2 full-page ID error to show three troubleshooting options#5729
aduth merged 3 commits intomainfrom
aduth-lg-5468-doc-auth-tips

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Dec 20, 2021

Why: So that we are consistent in the options that we show users during document capture.

Screenshots:

Before After
image Screen Shot 2021-12-20 at 8 28 10 AM

Comment on lines 17 to 18
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, instead of overloading this usage to support either the element or the array of options, we could enforce the element be passed always. A difference is that we default a heading in the case that someone passes troubleshootingOptions but not troubleshootingHeading, but maybe that default could be moved to TroubleshootingOptions itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you're saying troubleshooting options would look like this?

{
  heading: 'aaa',
  options: [ { text, url  },  { text, url } ],
}

That makes sense to me, but the current overload is not so complex either

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was imagining to remove support for troubleshootingHeading and troubleshootingOptions as an array, and only to only support it passed as an element.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh so more like a children passthrough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh so more like a children passthrough?

Yeah, although Warning already accepts children for content of the warning, so we'd need it to be a different prop. I figure we can use the existing troubleshootingOptions for this.

So usage ends up being like:

<Warning
  troubleshootingOptions={<TroubleshootingOptions {/*...*/} />}
>
  Warning content
</Warning>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll plan to make this refactoring, but noting a couple watch items for the future:

  • If we're changing pairing props "troubleshootingHeading" and "troubleshootingOptions" to a single "options" prop, it raises a question of what to do about "actionText" and "actionOnClick", as they are similar in nature.
    • It's not as obvious to allow an element to be passed through for the action, since we want to enforce an opinionated appearance of the button ("big", "primary").
  • Noting that we have a similar ERB file for this markup, and options ideally align (idv/shared/_error.html.erb)
    • Proposed refactoring arguably brings into closer alignment, since "options" is a singular setting there, and "heading" has smart defaults.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aduth
Copy link
Contributor Author

aduth commented Dec 21, 2021

96e2e47b7 refactors to always accept an element as Warning's troubleshootingOptions prop value. This also incorporates design feedback to use the same set of troubleshooting options for pre-submit blur / glare tips (see related Slack discussion). f7193a5 is a temporary commit cherry-picked from a necessary localization fix implemented separately at #5740.

@aduth aduth force-pushed the aduth-lg-5468-doc-auth-tips branch from 4b03487 to cde12aa Compare December 21, 2021 18:13
@aduth aduth merged commit 6a02b75 into main Dec 22, 2021
@aduth aduth deleted the aduth-lg-5468-doc-auth-tips branch December 22, 2021 14:17
jmhooper pushed a commit that referenced this pull request Dec 28, 2021
… options (#5729)

* LG-5468: Update IAL2 full-page ID error to show three troubleshooting options

**Why**: So that we are consistent in the options that we show users during document capture.

* Refactor to always pass troubleshooting options to warning as element

* Customize troubleshooting header text for CaptureTips

https://gsa-tts.slack.com/archives/CNCGEHG1G/p1640103718016000?thread_ts=1640033914.012600&cid=CNCGEHG1G
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants