Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: Issue 25818 Preview Area Weird Boarder Accessibility Addon #28844

Open
wants to merge 5 commits into
base: next
Choose a base branch
from
Open

Fixed: Issue 25818 Preview Area Weird Boarder Accessibility Addon #28844

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa7a074
Applied filters to iframe contents rather than iframe itself
saadmalik96 Aug 2, 2024
8e8aa0e
Merge branch 'storybookjs:next' into weird-border-fix
RuxueJ Aug 8, 2024
8d6f1ff
Merge branch 'next' into weird-border-fix
saadmalik96 Aug 12, 2024
c0a098b
Merge branch 'next' into weird-border-fix
saadmalik96 Aug 13, 2024
80d4786
Merge branch 'storybookjs:next' into weird-border-fix
saadmalik96 Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 32 additions & 24 deletions code/addons/a11y/src/components/VisionSimulator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import type { ReactNode } from 'react';
import React, { useState } from 'react';

import React, { useState, useEffect } from 'react';
import ReactDOMServer from 'react-dom/server';
import { IconButton, TooltipLinkList, WithTooltip } from 'storybook/internal/components';
import { Global, styled } from 'storybook/internal/theming';

import { AccessibilityIcon } from '@storybook/icons';

import { Filters } from './ColorFilters';

const iframeId = 'storybook-preview-iframe';
const iframeContents = 'storybook-root';

interface Option {
name: string;
Expand Down Expand Up @@ -42,14 +41,6 @@ const getFilter = (filterName: string) => {
return `url('#${filterName}')`;
};

const Hidden = styled.div(() => ({
'&, & svg': {
position: 'absolute',
width: 0,
height: 0,
},
}));

const ColorIcon = styled.span<{ filter: string }>(
{
background: 'linear-gradient(to right, #F44336, #FF9800, #FFEB3B, #8BC34A, #2196F3, #9C27B0)',
Expand Down Expand Up @@ -121,19 +112,39 @@ const getColorList = (active: Filter, set: (i: Filter) => void): Link[] => [
}),
];

const hiddenStyles = {
position: 'absolute',
width: '0',
height: '0',
};

const injectFilters = (iframeDocument: Document) => {
if (!iframeDocument.getElementById('color-filters')) {
const filtersContainer = iframeDocument.createElement('div');
filtersContainer.id = 'color-filters';
Object.assign(filtersContainer.style, hiddenStyles);
iframeDocument.body.appendChild(filtersContainer);
const filtersSvgString = ReactDOMServer.renderToStaticMarkup(<Filters />);
filtersContainer.innerHTML = filtersSvgString;
}
};

export const VisionSimulator = () => {
const [filter, setFilter] = useState<Filter>(null);

useEffect(() => {
const iframe = document.getElementById(iframeId) as HTMLIFrameElement;
if (iframe && iframe.contentDocument) {
Copy link
Member

Choose a reason for hiding this comment

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

Accessing the contentDocument will only work if the iframe is on the same origin, which is not always the case.

We should encapsulate this behavior in a storybook decorator instead, similar to how the backgrounds addon works.

Syncing state via a global.

storybook/code/addons/backgrounds/src/decorator.ts

Line 23 in 513bb66

export const withBackgroundAndGrid = (

Copy link

Choose a reason for hiding this comment

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

Hi @ndelangen thank you for taking a look at our PR and for leaving such a detailed response.

In the issue page, we left a simpler alternate solution which is to remove the seemingly unnecessary boxShadow css in iframe.tsx. The boxShadow has no visual effect on preview window at the moment and since the blur filter only causes the bug with this boxShadow, this would resolve the weird border bug and then upgrading the addon can be a separate issue? You can find this here:

storybook/code/core/src/manager/components/preview/Iframe.tsx

Line 17 in 4bbfb14

boxShadow: '0 0 100px 100vw rgba(0,0,0,0.5)',

If you still prefer upgrading the addon for this particular issue, could you roughly outline the steps needed and your expectation? Like would the a11y addon need to be structured similarly to the background addon? We would like to understand this approach a bit better before diving right in.

injectFilters(iframe.contentDocument);
const storyRoot = iframe.contentDocument.getElementById(iframeContents);
if (storyRoot) {
storyRoot.style.filter = filter ? getFilter(filter.name) : 'none';
}
}
}, [filter]);

return (
<>
{filter && (
<Global
styles={{
[`#${iframeId}`]: {
filter: getFilter(filter.name),
},
}}
/>
)}
<WithTooltip
placement="top"
tooltip={({ onHide }) => {
Expand All @@ -150,9 +161,6 @@ export const VisionSimulator = () => {
<AccessibilityIcon />
</IconButton>
</WithTooltip>
<Hidden>
<Filters />
</Hidden>
</>
);
};