fix: add banner for unresponsive iframes #6330
Conversation
|
|
||
| export const IframeSkippedWarning = NamedFC('IframeSkippedWarning', () => ( | ||
| <div data-automation-id={IframeSkippedWarningContainerAutomationId}> | ||
| There are iframes in the target page that are non-responsive. These frames have been skipped |
There was a problem hiding this comment.
This is a good starting point, but I'm not sure this is actionable enough. What do we want a user who sees this warning to do about it, exactly? Perhaps we should include something like "If you see this warning consistently for a page where it isn't expected, please file an issue."? @ferBonnin , what are your thoughts?
There was a problem hiding this comment.
Or perhaps it would be more manageable to create a single known tracking issue and link to it, asking folks to comment on it with the site in question.
There was a problem hiding this comment.
Either of those options sound reasonable to me! I'll defer to @ferBonnin's judgement for specific wording/ how to manage issues.
There was a problem hiding this comment.
let's create a tracking issue and let users comment on it, the barrier is lower for the user.
What are the cases where the user would get this message unexpectedly? if there aren't iframes?
for the message, how about:
If you get this warning in a page (due to reason to get it unexpectedly), please [let us know](link to the issue).
There was a problem hiding this comment.
What are the cases where the user would get this message unexpectedly? if there aren't iframes?
Users shouldn't get this message when there aren't iframes on the page, but they might get it on pages where they expect the iframes to be responsive but they're not, for example, if the iframes are responding but very slowly.
for the message, how about:
If you get this warning in a page , please [let us know](link to the issue).
Sounds good to me! I assume that would be in addition to the existing text?
There was a problem hiding this comment.
not sure I follow completely: in which cases would we want the user to let us know / file a bug?
Sounds good to me! I assume that would be in addition to the existing text?
edited my message because I was using <> and the text between them didn't show 🙂
Yes, that would be in addition to the text " There are iframes in the target page ..."
There was a problem hiding this comment.
Not sure if Dan has other thoughts, but IMO I think it would be interesting to get some example target pages where a user thinks that the iframes should be responsive but for some reason we aren't able to communicate with them (and as a result show this error). So it would be valuable for a user to file an issue if they are seeing this warning unexpectedly and think that the iframes should be/ are responsive.
In this case, should we be a little more specific in the wording and include something like If you get this warning unexpectedly in a page where you expect the iframes to be responsive, please [let us know](link to the issue).
There was a problem hiding this comment.
I created a tracking issue to link to here: #6347, please feel free to edit it directly if I missed anything!
There was a problem hiding this comment.
That message looks great to me!
There was a problem hiding this comment.
LGTM! I think the most common case will actually be that the person running the tool won't really know a lot of details about the iframes causing problems in the page, but I think that message should cover that case okay. Thanks for the update!
Details
This PR adds logic to detect when iframes have been skipped in an axe scan via that frames-tested rule. When iframes have been skipped, it shows a yellow banner to notify the user.
Motivation
Addresses issue #6128
Context
Screenshot of banner with fastpass (shows up similarly in assessment UI):
[Screenshot description: Yellow banner across top of details view with text "there are iframes in the target page that are non-responsive. These frames have been skipped and are not included in the result.]
Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). SeeCONTRIBUTING.md.