onc-healthit / onc-certification-g10-test-kit

ONC Certification (g)(10) Standardized API Tests
Apache License 2.0
34 stars 11 forks source link

Minor Visual Fixes #447

Closed bmath10 closed 1 year ago

bmath10 commented 1 year ago

JIRA Ticket: https://oncprojectracking.healthit.gov/support/browse/FI-2011

To Test:

Question: Do you like the change to the modal, and if so, should I make a follow up ticket to change the other modals across test kits to match this?

arscan commented 1 year ago

For reference, this is what that modal looks like before:

Screenshot 2023-07-21 at 10 39 32 AM

Though, not quite 100% apples-to-apples comparison as in my example the domain is for inferno.healthit.gov (which is what most users will see, as most users just use that instance) and not localhost

I don't personally think that either are great visually. The updated format is still a bit weird to me, as now there is a sentence that is continued across multiple breaks... so on balance that seems to negate any improvements in line-wrapping? I can't come up with any easy solutions off the top of my head.

Functionally the info is mostly 'fyi' as those values should be known by the system under test (we just found that most people don't read the instructions earlier so get stuck and appreciate a hint of when is supposed to happen next). So we don't expect users to copy/paste those values, though we don't disallow to just help systems get through to the next step.

Personally, a very low priority option that might be nice is an expando kinda thing that github provides, so you could do soemthing like this....

Inferno App Redirect Information **Redirect URL:** http://blah.blah.blah **State:** 343434-23434-23423234

But I definitely don't think this should be a priority for this kind of thing. Maybe in the general vein of 'improving our markdown capabilities' as a more substantial ticket.

Jammjammjamm commented 1 year ago

I'm pretty neutral on that specific change, but I don't like that it requires allowing trailing whitespace. It's pretty normal for trailing whitespace to get automatically removed, so I think requiring it could become annoying.

bmath10 commented 1 year ago

@arscan that's fair, I didn't think it looked great visually either, but I couldn't think of another way to format it easily that made it more legible (since the current way sort of just looks like one long URL at a glance to me). I'm happy to revert my change and make a separate ticket to address markdown formatting if that's how you want to move forward!

bmath10 commented 1 year ago

@Jammjammjamm yeah I didn't like having to turn off the linter for those lines to allow for the trailing whitespace. Seems like markdown should have a better way of doing a single line enter that doesn't require the whitespace...

bmath10 commented 1 year ago

@arscan @Jammjammjamm : reverted the change to the modal and added this low priority ticket for potential future change: https://oncprojectracking.healthit.gov/support/browse/FI-2044

Let me know if there's anything else I need to change!