Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-embedded-content-consumer] - Show focus indicator on iframe of the embedded content when it has focus #1827

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
368c3fc
Set visual focus indicator on the frame when the frame has focus
kolkheang Aug 29, 2023
47a1105
WIP - trying out different scenario
kolkheang Sep 26, 2023
2685b29
trying different scenario. Allow all hosts in webpack config to test …
kolkheang Sep 28, 2023
836bd5c
WIP - trying out another approach to set style on the frame
kolkheang Sep 28, 2023
5e8e4c8
WIP - setting the frame's style when focus/blur event happens on the …
kolkheang Oct 2, 2023
4bedf98
WIP - rework the logic a to pull in the css parser string to object, …
kolkheang Oct 3, 2023
93d9aa3
Update xfc paths and remove some files no longer needed
kolkheang Oct 4, 2023
2e9e450
Update wdio tests
kolkheang Oct 5, 2023
3bf6808
Remove config to allowed host for dev server
kolkheang Oct 5, 2023
0727c53
Remove package-lock.json
kolkheang Oct 5, 2023
e04850d
Update wdio tests, add a few tests to show the outline style on the f…
kolkheang Oct 10, 2023
4b57dc3
Fix lint issues
kolkheang Oct 10, 2023
6f6d1d9
Read styles from scss file and update wdio tests
kolkheang Oct 10, 2023
49aa11d
Fix lint issue
kolkheang Oct 10, 2023
0ecd9dd
Update outline style for the 3 themes
kolkheang Oct 10, 2023
636f744
Fix paths for tests
kolkheang Oct 10, 2023
550ca80
Re-work the module scss file
kolkheang Oct 10, 2023
01d2771
Fix flaky tests
kolkheang Oct 11, 2023
f417dbd
Update CHANGELOG
kolkheang Oct 11, 2023
ec511e2
Merge branch 'main' into show-focus-indicator-on-iframe
kolkheang Oct 11, 2023
b1d1148
Update packages/terra-embedded-content-consumer/CHANGELOG.md
kolkheang Oct 12, 2023
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,6 @@
"webpack-cli": "^4.6.0",
"webpack-dev-server": "^4.11.1",
"webpack-merge": "^5.1.4",
"xfc": "^1.2.1"
"xfc": "git+https://github.com/kolkheang/xfc.git#display-frame-focus-indicator"
kolkheang marked this conversation as resolved.
Show resolved Hide resolved
}
}
3 changes: 3 additions & 0 deletions packages/terra-embedded-content-consumer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Added
* Added visual focus indicator (outline style) on the iframe when it has focus.

## 3.39.0 - (October 3, 2023)

* Added
Expand Down
5 changes: 3 additions & 2 deletions packages/terra-embedded-content-consumer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
"peerDependencies": {
"react": "^16.8.5",
"react-dom": "^16.8.5",
"xfc": "^1.2.1"
"xfc": "git+https://github.com/kolkheang/xfc.git#display-frame-focus-indicator"
sdadn marked this conversation as resolved.
Show resolved Hide resolved
},
"dependencies": {
"classnames": "^2.2.5",
"prop-types": "^15.5.8"
"prop-types": "^15.5.8",
"style-to-object": "^0.4.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not need to add this third-party dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. With the current way, there is just 1 style for focus outline, and one style for blur outline and we have two variables for these. We can set the value for each into the frame's style directly. Previously, I was thinking that we could set multiple style attributes for focus or blur.

},
"scripts": {
"compile": "babel --root-mode upward src --out-dir lib --copy-files",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Consumer } from 'xfc';
import parse from 'style-to-object';
import styles from './EmbeddedContentConsumer.module.scss';

const propTypes = {
/**
Expand Down Expand Up @@ -94,6 +96,10 @@ class EmbeddedContentConsumer extends React.Component {
Object.assign(frameOptions.iframeAttrs, { title: this.props.title });
}

const focusStyleStr = `outline: ${styles.focusOutline}`;
const blurStyleStr = `outline: ${styles.blurOutline}`;
frameOptions.focusIndicator = { focusStyleStr, blurStyleStr };

// Mount the provided source as the application into the content wrapper.
this.xfcFrame = Consumer.mount(this.embeddedContentWrapper, this.props.src, frameOptions);

Expand All @@ -102,6 +108,24 @@ class EmbeddedContentConsumer extends React.Component {
this.props.onMount(this.xfcFrame);
}

// Cover other scenario where xfc frame style doesn't apply
// such as when `srcdoc` attribute is used which doesn't work
// within xfc's JSONRPC communication.
this.xfcFrame?.iframe?.contentWindow?.addEventListener('focus', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The contentWindow gives you a reference to the window for the embedded content. You should be able to use document.documentElement (HTML tag) or document.body to handle the logic as necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'd like to handle the focus within the iframe element though, not everywhere else in the document body.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can handle by styling the iFrame element. How are you giving focus to your iFrame? Did you set a tabIndex?

const styleObj = parse(focusStyleStr);
Object.entries(styleObj).forEach(([key, value]) => {
this.xfcFrame.iframe.style[key] = value;
});
}, true);

// Listen for blur event and callback function to apply the style
this.xfcFrame?.iframe?.contentWindow?.addEventListener('blur', () => {
const styleObj = parse(blurStyleStr);
Object.entries(styleObj).forEach(([key, value]) => {
this.xfcFrame.iframe.style[key] = value;
});
}, true);
Comment on lines +114 to +127
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this logic, you should be able to use the ":focus" psuedo class.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you give an example on how to setup the style and apply to the iframe's style?

Copy link
Member Author

Choose a reason for hiding this comment

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

Like this?

iframe:focus {
  outline: 2px dashed #000;
}

Copy link
Contributor

@cm9361 cm9361 Oct 13, 2023

Choose a reason for hiding this comment

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

I would think that your iFrame would have an associated class name or id for styling. However, it would be a similar concept to what you have described here, yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

The :focus pseudo-class isn't working for iframe. I'll check to see if we can update the classname on the fly for switching the style.


// Attach the event handlers to the xfc frame.
this.addEventListener('xfc.launched', this.props.onLaunch);
this.addEventListener('xfc.authorized', this.props.onAuthorize);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Themes
@import './clinical-lowlight-theme/EmbeddedContentConsumer.module';
@import './orion-fusion-theme/EmbeddedContentConsumer.module';

:local {
/* stylelint-disable terra/custom-property-name */
$blur-outline: var(--terra-embedded-content-consumer-blur-outline, none);
$focus-outline: var(--terra-embedded-content-consumer-focus-outline, 2px dashed #000);

:export {
/* stylelint-disable property-no-unknown */
blurOutline: $blur-outline;
focusOutline: $focus-outline;
}
Comment on lines +7 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to set the styles using classes. I don't think that this is the correct implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are not for the classes to be applied to an html element like we normally do.

Copy link
Contributor

Choose a reason for hiding this comment

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

You would add the class to the iFrame element.

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';

import '../../../initalizeXFC';
import './ProviderIframe.module.scss';
import './ProviderTestTemplate.module.scss';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
:local {
.clinical-lowlight-theme {
--terra-embedded-content-consumer-focus-outline: 2px dashed #b2b5b6;
--terra-embedded-content-consumer-blur-outline: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need a blur outline style. That should just be the normal state of the element.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you saying we can hard-code outline: none to remove the style from the frame when there is no focus anymore? Because we set the outline style when there is focus. And if we don't remove that style, it stays.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I am saying that you would have CSS for the normal state. When the element is not focused, that CSS will no longer be applied.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
:local {
.orion-fusion-theme {
--terra-embedded-content-consumer-focus-outline: 2px solid #3496cf;
--terra-embedded-content-consumer-blur-outline: none;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -169,32 +169,34 @@ describe(EmbeddedContentConsumer, () => {

frame.unmount();
});
});

it('sets appropriate config option when resizeConfig.scrolling is true', () => {
const embeddedContentConsumer = (
<div>
<EmbeddedContentConsumer
src="https://www.google.com/"
options={{ resizeConfig: { scrolling: true } }}
/>
</div>
);

const wrapper = shallow(embeddedContentConsumer);
expect(wrapper).toMatchSnapshot();
});

it('sets appropriate config option when resizeConfig.scrolling is false', () => {
const embeddedContentConsumer = (
<div>
<EmbeddedContentConsumer
src="https://www.google.com/"
options={{ resizeConfig: { scrolling: false } }}
/>
</div>
);

const wrapper = shallow(embeddedContentConsumer);
expect(wrapper).toMatchSnapshot();
describe('scrolling', () => {
it('sets appropriate config option when resizeConfig.scrolling is true', () => {
const embeddedContentConsumer = (
<div>
<EmbeddedContentConsumer
src="https://www.google.com/"
options={{ resizeConfig: { scrolling: true } }}
/>
</div>
);

const wrapper = shallow(embeddedContentConsumer);
expect(wrapper).toMatchSnapshot();
});

it('sets appropriate config option when resizeConfig.scrolling is false', () => {
const embeddedContentConsumer = (
<div>
<EmbeddedContentConsumer
src="https://www.google.com/"
options={{ resizeConfig: { scrolling: false } }}
/>
</div>
);

const wrapper = shallow(embeddedContentConsumer);
expect(wrapper).toMatchSnapshot();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,35 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EmbeddedContentConsumer scrolling sets appropriate config option when resizeConfig.scrolling is false 1`] = `
<div>
<EmbeddedContentConsumer
options={
Object {
"resizeConfig": Object {
"scrolling": false,
},
}
}
src="https://www.google.com/"
/>
</div>
`;

exports[`EmbeddedContentConsumer scrolling sets appropriate config option when resizeConfig.scrolling is true 1`] = `
<div>
<EmbeddedContentConsumer
options={
Object {
"resizeConfig": Object {
"scrolling": true,
},
}
}
src="https://www.google.com/"
/>
</div>
`;

exports[`EmbeddedContentConsumer should render the embedded content consumer container 1`] = `
<div>
<EmbeddedContentConsumer
Expand Down Expand Up @@ -79,33 +109,3 @@ exports[`EmbeddedContentConsumer should render the embedded content consumer wit
</EmbeddedContentConsumer>
</div>
`;

exports[`sets appropriate config option when resizeConfig.scrolling is false 1`] = `
<div>
<EmbeddedContentConsumer
options={
Object {
"resizeConfig": Object {
"scrolling": false,
},
}
}
src="https://www.google.com/"
/>
</div>
`;

exports[`sets appropriate config option when resizeConfig.scrolling is true 1`] = `
<div>
<EmbeddedContentConsumer
options={
Object {
"resizeConfig": Object {
"scrolling": true,
},
}
}
src="https://www.google.com/"
/>
</div>
`;
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
});

it('Provider triggers EventA message', () => {
const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/custom-event-provider"]');
const myFrame = $('iframe[id="custom-event-consumer-frame"]');
browser.switchToFrame(myFrame);

$('#EventA').click();
Expand All @@ -30,7 +30,7 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
});

it('Provider triggers EventA message', () => {
const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/custom-events-provider"]');
const myFrame = $('iframe[id="custom-events-consumer-frame"]');
browser.switchToFrame(myFrame);

$('#EventA').click();
Expand All @@ -40,15 +40,15 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
});

it('successfully replied with EventA message', () => {
const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/custom-events-provider"]');
const myFrame = $('iframe[id="custom-events-consumer-frame"]');
browser.switchToFrame(myFrame);

expect($('#embedded-content-consumer-reply')).toHaveTextContaining('eventA');
browser.switchToParentFrame();
});

it('Provider triggers EventB message', () => {
const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/custom-events-provider"]');
const myFrame = $('iframe[id="custom-events-consumer-frame"]');
browser.switchToFrame(myFrame);

$('#EventB').click();
Expand All @@ -58,7 +58,7 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
});

it('successfully replied with EventB message', () => {
const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/custom-events-provider"]');
const myFrame = $('iframe[id="custom-events-consumer-frame"]');
browser.switchToFrame(myFrame);

expect($('#embedded-content-consumer-reply')).toHaveTextContaining('eventB');
Expand All @@ -69,9 +69,9 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
it('has mounted, launched, and authorized elements', () => {
browser.url('/raw/tests/cerner-terra-framework-docs/embedded-content-consumer/consumers/data-status-consumer');
const timeout = browser.options.waitforTimeout + 5000;
$('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/data-status-provider"]').waitForDisplayed({ timeout });
$('iframe[id="data-embedded-consumer-data-status"]').waitForDisplayed({ timeout });

const myFrame = $('iframe[src="/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/data-status-provider"]');
const myFrame = $('iframe[id="data-embedded-consumer-data-status"]');
browser.switchToFrame(myFrame);

expect($('#Mounted').isExisting());
Expand Down Expand Up @@ -105,4 +105,29 @@ Terra.describeViewports('Embedded Content Consumer', ['tiny', 'large'], () => {
Terra.validates.element('Scroll content into view');
});
});

describe('visual focus indicator', () => {
it('shows visual focus indicator on the frame when clicked', () => {
browser.url('/raw/tests/cerner-terra-framework-docs/embedded-content-consumer/consumers/basic-consumer-with-scrolling');
$('iframe[id="basic-consumer-with-scrolling"]').waitForDisplayed();

const myFrame = $('iframe[id="basic-consumer-with-scrolling"]');
browser.switchToFrame(myFrame);

const p = $('<p>');
p.scrollIntoView();
p.click();

browser.switchToParentFrame();

Terra.validates.element('focus indicator on the frame when clicked', { selector: '#site' });
});

it('shows visual focus indicator on the frame when tabbing through and focus is on the frame', () => {
browser.url('/raw/tests/cerner-terra-framework-docs/embedded-content-consumer/consumers/basic-consumer');
browser.keys('Tab');

Terra.validates.element('focus indicator on the frame when tabbing through', { selector: '#site' });
});
});
});
3 changes: 3 additions & 0 deletions packages/terra-framework-docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Added
* Added documentation for FlowsheetDataGrid in `terra-data-grid`.

* Updated
* Updated examples, and tests to show the visual focus indicator on the iframe of the embedded content
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We can add a . at the EOL.


## 1.39.0 - (October 3, 2023)

* Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import { Consumer } from 'xfc';
import '../providers/EmbeddedContentConsumerCommon.module.scss';

Consumer.init();

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we keep the newline here or remove it from the other doc files as well?

const BasicConsumer = () => (
<EmbeddedContentConsumer
src="/terra-framework/#/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/basic-provider"
title="Basic content example"
options={{
resizeConfig: { scrolling: true },
}}
/>
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import EmbeddedContentConsumer from 'terra-embedded-content-consumer';
import { Consumer } from 'xfc';
import '../providers/EmbeddedContentConsumerCommon.module.scss';

Consumer.init();

Expand All @@ -9,7 +10,7 @@ const BasicConsumerWithScrolling = () => (
src="/terra-framework/#/raw/provider/cerner-terra-framework-docs/embedded-content-consumer/providers/basic-provider"
title="Basic content example of scrolling is enabled"
options={{
resizeConfig: { scrolling: true, fixedWidth: '100%', fixedHeight: '100px' },
resizeConfig: { scrolling: true, fixedWidth: '100%', fixedHeight: '120px' },
}}
/>
);
Expand Down
Loading
Loading