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.

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

Highlight CSS and JS selectors #1598

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
695 changes: 658 additions & 37 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@
"bugs": "https://trello.com/b/ONaFg6wh/popcode",
"license": "MIT",
"dependencies": {
"@babel/parser": "^7.1.3",
"@babel/traverse": "^7.1.4",
"@babel/types": "^7.1.3",
"@firebase/app": "^0.3.4",
"@firebase/app-types": "^0.3.2",
"@firebase/auth": "^0.7.6",
Expand Down
11 changes: 11 additions & 0 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import {
} from './projects';

import {
currentCursorChanged,
focusLine,
editorBlurred,
editorFocused,
editorFocusedRequestedLine,
startDragColumnDivider,
stopDragColumnDivider,
Expand Down Expand Up @@ -74,6 +77,10 @@ import {
updateResizableFlex,
} from './resizableFlex';

import {
updateSelectorLocations,
} from './selectorLocations';

export {
clearConsoleEntries,
consoleInputChanged,
Expand All @@ -93,13 +100,16 @@ export {
unhideComponent,
toggleComponent,
focusLine,
editorBlurred,
editorFocused,
editorFocusedRequestedLine,
previousConsoleHistory,
nextConsoleHistory,
startDragColumnDivider,
stopDragColumnDivider,
notificationTriggered,
userDismissedNotification,
currentCursorChanged,
updateNotificationMetadata,
exportProject,
projectExportDisplayed,
Expand All @@ -126,4 +136,5 @@ export {
updateResizableFlex,
startAccountMigration,
dismissAccountMigration,
updateSelectorLocations,
};
7 changes: 7 additions & 0 deletions src/actions/selectorLocations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import {createAction} from 'redux-actions';

export const updateSelectorLocations = createAction(
'UPDATE_SELECTOR_LOCATIONS',
selectors => ({selectors}),
(_selectors, timestamp = Date.now()) => ({timestamp}),
);
18 changes: 18 additions & 0 deletions src/actions/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@ export const editorFocusedRequestedLine = createAction(
'EDITOR_FOCUSED_REQUESTED_LINE',
);

export const editorFocused = createAction(
'EDITOR_FOCUSED',
(source, cursor, language) => ({source, cursor, language}),
);

export const editorBlurred = createAction(
'EDITOR_BLURRED',
);

export const startDragColumnDivider = createAction(
'START_DRAG_COLUMN_DIVIDER',
);
Expand All @@ -34,6 +43,15 @@ export const userDismissedNotification = createAction(
type => ({type}),
);

export const currentCursorChanged = createAction(
'CURRENT_CURSOR_CHANGED',
(source, cursor, language) => ({source, cursor, language}),
);

export const currentFocusedSelectorChanged = createAction(
'CURRENT_FOCUSED_SELECTOR_CHANGED',
);

export const updateNotificationMetadata = createAction(
'UPDATE_NOTIFICATION_METADATA',
(type, metadata) => ({type, metadata}),
Expand Down
21 changes: 21 additions & 0 deletions src/components/Editor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,27 @@ class Editor extends React.Component {

_startNewSession(source) {
const session = createAceSessionWithoutWorker(this.props.language, source);
const cursor = session.selection.lead;
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like lead as a property may not be part of the public API—the docs only mention getSelectionLead()

session.on('change', () => {
this.props.onInput(this._editor.getValue());
});
session.selection.on('changeCursor', () => {
this.props.onCursorChange(
this._editor.getValue(),
cursor,
this.props.language,
);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

In general I think facts about UI state should either be completely invisible to application (redux) state, or completely controlled by/synchronized with Redux. So in this case, once application state is concerned with the cursor position, it should also control cursor position. Practically I think this just means we should be taking cursor position and focus as props in this component, watching for changes, and updating the editor components if they’re out of sync.

This also means we can replace the never-terribly-appealing REQUEST_FOCUSED_LINE mechanism with a simple action to move the cursor to a particular location at the Redux level.

this._editor.on('blur', () => {
this.props.onEditorBlurred();
});
this._editor.on('focus', () => {
this.props.onEditorFocused(
this._editor.getValue(),
cursor,
this.props.language,
);
});
session.setAnnotations(this.props.errors);
this._editor.setSession(session);
this._editor.moveCursorTo(0, 0);
Expand All @@ -147,6 +165,9 @@ Editor.propTypes = {
requestedFocusedLine: PropTypes.instanceOf(EditorLocation),
source: PropTypes.string.isRequired,
textSizeIsLarge: PropTypes.bool.isRequired,
onCursorChange: PropTypes.func.isRequired,
onEditorBlurred: PropTypes.func.isRequired,
onEditorFocused: PropTypes.func.isRequired,
onInput: PropTypes.func.isRequired,
onRequestedLineFocused: PropTypes.func.isRequired,
};
Expand Down
9 changes: 9 additions & 0 deletions src/components/EditorsColumn.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ export default function EditorsColumn({
requestedFocusedLine,
onComponentHide,
onComponentUnhide,
onEditorBlurred,
onEditorCursorChange,
onEditorFocused,
onEditorInput,
onRef,
onRequestedLineFocused,
Expand Down Expand Up @@ -70,6 +73,9 @@ export default function EditorsColumn({
requestedFocusedLine={requestedFocusedLine}
source={currentProject.sources[language]}
textSizeIsLarge={isTextSizeLarge}
onCursorChange={onEditorCursorChange}
onEditorBlurred={onEditorBlurred}
onEditorFocused={onEditorFocused}
onInput={partial(
onEditorInput,
currentProject.projectKey,
Expand Down Expand Up @@ -144,6 +150,9 @@ EditorsColumn.propTypes = {
style: PropTypes.object.isRequired,
onComponentHide: PropTypes.func.isRequired,
onComponentUnhide: PropTypes.func.isRequired,
onEditorBlurred: PropTypes.func.isRequired,
onEditorCursorChange: PropTypes.func.isRequired,
onEditorFocused: PropTypes.func.isRequired,
onEditorInput: PropTypes.func.isRequired,
onRef: PropTypes.func.isRequired,
onRequestedLineFocused: PropTypes.func.isRequired,
Expand Down
7 changes: 7 additions & 0 deletions src/components/Preview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import PreviewFrame from './PreviewFrame';
export default function Preview({
compiledProjects,
consoleEntries,
focusedSelector,
showingErrors,
onConsoleError,
onConsoleLog,
Expand All @@ -29,6 +30,7 @@ export default function Preview({
<PreviewFrame
compiledProject={compiledProject}
consoleEntries={consoleEntries}
focusedSelector={focusedSelector}
isActive={key === compiledProjects.keySeq().last()}
key={compiledProject.compiledProjectKey}
onConsoleError={onConsoleError}
Expand Down Expand Up @@ -63,6 +65,7 @@ export default function Preview({
Preview.propTypes = {
compiledProjects: ImmutablePropTypes.iterable.isRequired,
consoleEntries: ImmutablePropTypes.iterable.isRequired,
focusedSelector: PropTypes.string,
showingErrors: PropTypes.bool.isRequired,
onConsoleError: PropTypes.func.isRequired,
onConsoleLog: PropTypes.func.isRequired,
Expand All @@ -71,3 +74,7 @@ Preview.propTypes = {
onRefreshClick: PropTypes.func.isRequired,
onRuntimeError: PropTypes.func.isRequired,
};

Preview.defaultProps = {
focusedSelector: null,
};
13 changes: 13 additions & 0 deletions src/components/PreviewFrame.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class PreviewFrame extends React.Component {
const {consoleEntries, isActive} = this.props;

if (this._channel && isActive) {
this._postFocusedSelectorToFrame(this.props.focusedSelector);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we only do this if the focused selector has in fact changed?

for (const [key, {expression}] of consoleEntries) {
if (!prevConsoleEntries.has(key) && expression) {
this._evaluateConsoleExpression(key, expression);
Expand Down Expand Up @@ -135,6 +136,13 @@ class PreviewFrame extends React.Component {
this.props.onConsoleLog(printedValue, compiledProjectKey);
}

_postFocusedSelectorToFrame(selector) {
this._channel.notify({
method: 'highlightElement',
params: selector,
});
}

_attachToFrame(frame) {
if (!frame) {
if (this._channel) {
Expand Down Expand Up @@ -168,11 +176,16 @@ class PreviewFrame extends React.Component {
PreviewFrame.propTypes = {
compiledProject: PropTypes.instanceOf(CompiledProjectRecord).isRequired,
consoleEntries: ImmutablePropTypes.iterable.isRequired,
focusedSelector: PropTypes.string,
isActive: PropTypes.bool.isRequired,
onConsoleError: PropTypes.func.isRequired,
onConsoleLog: PropTypes.func.isRequired,
onConsoleValue: PropTypes.func.isRequired,
onRuntimeError: PropTypes.func.isRequired,
};

PreviewFrame.defaultProps = {
focusedSelector: null,
};

export default PreviewFrame;
30 changes: 30 additions & 0 deletions src/containers/EditorsColumn.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ import {
isTextSizeLarge,
} from '../selectors';
import {
currentCursorChanged,
editorFocusedRequestedLine,
hideComponent,
updateProjectSource,
unhideComponent,
editorBlurred,
editorFocused,
} from '../actions';

function mapStateToProps(state) {
Expand Down Expand Up @@ -41,6 +44,33 @@ function mapDispatchToProps(dispatch) {
onRequestedLineFocused() {
dispatch(editorFocusedRequestedLine());
},

onEditorCursorChange(source, cursor, language) {
dispatch(
currentCursorChanged(
source,
cursor,
language,
),
);
},

onEditorBlurred() {
dispatch(
editorBlurred(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this still contain a language argument? As it is I think we rely on ACE dispatching the blur event before the focus event in the case where the user clicks from one editor to another.

);
},

onEditorFocused(source, cursor, language) {
dispatch(
editorFocused(
source,
cursor,
language,
),
);
},

};
}

Expand Down
2 changes: 2 additions & 0 deletions src/containers/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import {
getCompiledProjects,
getConsoleHistory,
getFocusedSelector,
isCurrentProjectSyntacticallyValid,
isUserTyping,
} from '../selectors';
Expand All @@ -22,6 +23,7 @@ function mapStateToProps(state) {
return {
compiledProjects: getCompiledProjects(state),
consoleEntries: getConsoleHistory(state),
focusedSelector: getFocusedSelector(state),
showingErrors: (
!isUserTyping(state) &&
!isCurrentProjectSyntacticallyValid(state)
Expand Down
3 changes: 3 additions & 0 deletions src/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import handleConsoleExpressions
from './previewSupport/handleConsoleExpressions';
import handleConsoleLogs from './previewSupport/handleConsoleLogs';
import overrideAlert from './previewSupport/overrideAlert';
import handleElementHighlights from './previewSupport/handleElementHighlights';

handleErrors();
handleConsoleExpressions();
handleConsoleLogs();
overrideAlert();
handleElementHighlights();

65 changes: 65 additions & 0 deletions src/previewSupport/handleElementHighlights.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import throttle from 'lodash-es/throttle';

import channel from './channel';

const RESIZE_THROTTLE = 250;
let highlightSelector = null;

const handleWindowResize = throttle(() => {
updateCovers(highlightSelector);
}, RESIZE_THROTTLE);

window.addEventListener('resize', handleWindowResize);

function getOffsetFromBody(element) {
if (element === document.body) {
return {top: 0, left: 0};
}
const {top, left} = getOffsetFromBody(element.offsetParent);
return {top: top + element.offsetTop, left: left + element.offsetLeft};
}

function removeCovers() {
const highlighterElements =
document.querySelectorAll('.__popcode-highlighter');
for (const highlighterElement of highlighterElements) {
highlighterElement.remove();
}
}

function addCovers(selector) {
const elements = document.querySelectorAll(selector);
for (const element of elements) {
const cover = document.createElement('div');
const rect = element.getBoundingClientRect();
let offset = {top: rect.top, left: rect.left};
if (element.offsetParent === null) {
cover.style.position = 'fixed';
} else if (element !== document.body ||
element !== document.documentElement) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like every element is either not <html> or not <body>?

offset = getOffsetFromBody(element);
}
document.body.appendChild(cover);
cover.classList = '__popcode-highlighter';
cover.style.left = `${offset.left}px`;
cover.style.top = `${offset.top}px`;
cover.style.width = `${element.offsetWidth}px`;
cover.style.height = `${element.offsetHeight}px`;
cover.classList.add('fade');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a second class here? Can’t we use the __popcode-highlighter class for all the rules we want to apply?

}
}

function updateCovers(selector) {
removeCovers();
if (selector !== null) {
highlightSelector = selector;
addCovers(selector);
}
}

export default function handleElementHighlights() {
channel.bind(
'highlightElement',
(_trans, selector) => updateCovers(selector),
);
}
6 changes: 6 additions & 0 deletions src/records/SelectorLocations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {Record, List} from 'immutable';

export default Record({
javascript: new List(),
css: new List(),
}, 'SelectorLocations');
1 change: 1 addition & 0 deletions src/records/UiState.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ export default Record({
openTopBarMenu: null,
requestedFocusedLine: null,
saveIndicatorShown: false,
focusedSelector: null,
}, 'UiState');
Loading