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

make open editor buttons linked #2558

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
46 changes: 45 additions & 1 deletion src/components/Button/Button.jsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,59 @@
import classNames from "classnames";
import PropTypes from "prop-types";
import React from "react";
import { getEditorWindowReference, setEditorWindowReference } from "../../services/Editor/Editor";

/**
* Basic button component that includes proper styling for disabled state
* and optional link wrapping
*/
export default function Button({ className, disabled = false, children, ...otherProps }) {
export default function Button({
className,
disabled = false,
children,
href,
onClick,
...otherProps
}) {
// If href is provided, wrap the button in an anchor tag
if (href && !disabled) {
return (
<a
href={href}
target="_blank"
rel="noopener noreferrer"
disabled={disabled}
{...otherProps}
className={classNames("mr-button", className, {
"mr-opacity-50 mr-cursor-not-allowed": disabled,
})}
onClick={(e) => {
e.preventDefault();
if (onClick) {
const existingEditor = getEditorWindowReference();
if (existingEditor && !existingEditor.closed) {
existingEditor.focus();
onClick(e);
} else {
const newWindow = window.open(href, "_blank");
setEditorWindowReference(newWindow);
onClick(e);
}
}
}}
>
{children}
</a>
);
}

return (
<button
className={classNames("mr-button", className, {
"mr-opacity-50 mr-cursor-not-allowed": disabled,
})}
disabled={disabled}
onClick={onClick}
{...otherProps}
>
{children}
Expand All @@ -23,4 +65,6 @@ Button.propTypes = {
className: PropTypes.string,
children: PropTypes.node.isRequired,
disabled: PropTypes.bool,
href: PropTypes.string,
onClick: PropTypes.func,
};
45 changes: 40 additions & 5 deletions src/components/UserEditorSelector/UserEditorSelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
import _pick from "lodash/pick";
import { Component } from "react";
import { FormattedMessage } from "react-intl";
import { DEFAULT_EDITOR, Editor, editorLabels, keysByEditor } from "../../services/Editor/Editor";
import {
DEFAULT_EDITOR,
Editor,
constructEditorUri,
editorLabels,
keysByEditor,
} from "../../services/Editor/Editor";
import { OPEN_STREET_MAP } from "../../services/VisibleLayer/LayerSources";
import BusySpinner from "../BusySpinner/BusySpinner";
import Button from "../Button/Button";
import Dropdown from "../Dropdown/Dropdown";
Expand All @@ -19,6 +26,21 @@
isSaving: false,
};

getEditorUri = (editor) => {
const { task, mapBounds, source, showMapillaryLayer, taskBundle, taskFeatureProperties } =

Check warning on line 30 in src/components/UserEditorSelector/UserEditorSelector.jsx

View workflow job for this annotation

GitHub Actions / build (18)

'taskFeatureProperties' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 30 in src/components/UserEditorSelector/UserEditorSelector.jsx

View workflow job for this annotation

GitHub Actions / build (20)

'taskFeatureProperties' is assigned a value but never used. Allowed unused vars must match /^_/u
this.props;

if (!task) return null;

const comment = task.parent?.checkinComment;
const options = {
imagery: source?.id !== OPEN_STREET_MAP ? source : undefined,
photoOverlay: showMapillaryLayer ? "mapillary" : null,
};

return constructEditorUri(editor, task, mapBounds, options, taskBundle, comment);
};

/** Process keyboard shortcuts for the edit controls */
handleKeyboardShortcuts = (event) => {
// Ignore if shortcut group is not active
Expand Down Expand Up @@ -84,7 +106,9 @@
}

chooseEditor = (editor, closeDropdown) => {
const updatedSettings = Object.assign({}, this.props.user.settings, { defaultEditor: editor });
const updatedSettings = Object.assign({}, this.props.user.settings, {
defaultEditor: editor,
});

this.setState({ isSaving: true });
this.props
Expand All @@ -95,19 +119,22 @@

render() {
const localizedEditorLabels = editorLabels(this.props.intl);
const currentEditor = this.currentEditor();
const editorUri = this.getEditorUri(currentEditor);

return (
<div className="mr-text-xs mr-text-white mr-flex mr-whitespace-nowrap mr-items-center">
<div className="mr-flex">
<Button
className="mr-button--green-fill mr-px-2 mr-cursor-pointer mr-text-sm"
onClick={() => this.props.pickEditor({ value: this.currentEditor() })}
onClick={() => this.props.pickEditor({ value: currentEditor })}
href={editorUri}
style={{ minWidth: "11.5rem" }}
>
{this.state.isSaving ? (
<BusySpinner />
) : (
localizedEditorLabels[keysByEditor[this.currentEditor()]] || (
localizedEditorLabels[keysByEditor[currentEditor]] || (
<FormattedMessage {...messages.editLabel} />
)
)}
Expand All @@ -128,12 +155,14 @@
)}
dropdownContent={(dropdown) => (
<ListEditorItems
{...this.props}
allowedEditors={this.props.allowedEditors}
editorLabels={localizedEditorLabels}
activeEditor={this.currentEditor()}
activeEditor={currentEditor}
chooseEditor={this.chooseEditor}
closeDropdown={dropdown.closeDropdown}
pickEditor={this.props.pickEditor}
getEditorUri={this.getEditorUri}
/>
)}
/>
Expand All @@ -150,6 +179,7 @@
chooseEditor,
closeDropdown,
pickEditor,
getEditorUri,
}) => {
const renderEditorItems = (isAllowed) =>
_compact(
Expand All @@ -160,6 +190,8 @@
const isEditorAllowed = !allowedEditors || allowedEditors.includes(editor);
if (isEditorAllowed !== isAllowed) return null;

const editorUri = getEditorUri(editor);

return (
<li key={editor} className={classNames({ active: editor === activeEditor })}>
<a
Expand All @@ -168,6 +200,9 @@
? chooseEditor(editor, closeDropdown)
: pickEditor({ value: editor })
}
href={editorUri}
target={editorUri ? "_blank" : undefined}
rel={editorUri ? "noopener noreferrer" : undefined}
>
<div className="mr-flex mr-items-center">
{!isEditorAllowed ? (
Expand Down
53 changes: 40 additions & 13 deletions src/services/Editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,9 @@
editorWindowReference.close();
}

if (editor === ID) {
editorWindowReference = window.open(
constructIdURI(task, mapBounds, options, taskBundle, replacedComment),
);
} else if (editor === LEVEL0) {
editorWindowReference = window.open(
constructLevel0URI(task, mapBounds, options, taskBundle, replacedComment),
);
} else if (editor === RAPID) {
editorWindowReference = window.open(
constructRapidURI(task, mapBounds, options, replacedComment),
);
}
editorWindowReference = window.open(
constructEditorUri(editor, task, mapBounds, options, taskBundle, replacedComment),
);

dispatch(editorOpened(editor, task.id, RequestStatus.success));
} else if (isJosmEditor(editor)) {
Expand Down Expand Up @@ -214,7 +204,7 @@
const baseUriComponent = `${window.env.REACT_APP_ID_EDITOR_SERVER_URL}?editor=id`;

const centerPoint = taskCenterPoint(mapBounds, task, taskBundle);
const mapUriComponent = "map=" + [mapBounds.zoom, centerPoint.lat, centerPoint.lng].join("/");

Check failure on line 207 in src/services/Editor/Editor.js

View workflow job for this annotation

GitHub Actions / build (18)

src/components/TaskPane/ActiveTaskDetails/ActiveTaskControls/TaskCompletionStep/TaskCompletionStep.test.jsx > TaskCompletionStep > renders task completion step 1 with required props

TypeError: Cannot read properties of undefined (reading 'zoom') ❯ constructIdURI src/services/Editor/Editor.js:207:47 ❯ Module.constructEditorUri src/services/Editor/Editor.js:756:14 ❯ UserEditorSelector.getEditorUri src/components/UserEditorSelector/UserEditorSelector.jsx:41:12 ❯ UserEditorSelector.render src/components/UserEditorSelector/UserEditorSelector.jsx:123:28 ❯ finishClassComponent node_modules/react-dom/cjs/react-dom.development.js:17485:31 ❯ updateClassComponent node_modules/react-dom/cjs/react-dom.development.js:17435:24 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:19073:16 ❯ HTMLUnknownElement.callCallback node_modules/react-dom/cjs/react-dom.development.js:3945:14 ❯ HTMLUnknownElement.callTheUserObjectsOperation node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30 ❯ innerInvokeEventListeners node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:350:25

Check failure on line 207 in src/services/Editor/Editor.js

View workflow job for this annotation

GitHub Actions / build (18)

src/components/TaskPane/ActiveTaskDetails/ActiveTaskControls/TaskCompletionStep/TaskCompletionStep.test.jsx > TaskCompletionStep > shows Edit button if allowedProgressions includes 1

TypeError: Cannot read properties of undefined (reading 'zoom') ❯ constructIdURI src/services/Editor/Editor.js:207:47 ❯ Module.constructEditorUri src/services/Editor/Editor.js:756:14 ❯ UserEditorSelector.getEditorUri src/components/UserEditorSelector/UserEditorSelector.jsx:41:12 ❯ UserEditorSelector.render src/components/UserEditorSelector/UserEditorSelector.jsx:123:28 ❯ finishClassComponent node_modules/react-dom/cjs/react-dom.development.js:17485:31 ❯ updateClassComponent node_modules/react-dom/cjs/react-dom.development.js:17435:24 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:19073:16 ❯ HTMLUnknownElement.callCallback node_modules/react-dom/cjs/react-dom.development.js:3945:14 ❯ HTMLUnknownElement.callTheUserObjectsOperation node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30 ❯ innerInvokeEventListeners node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:350:25

Check failure on line 207 in src/services/Editor/Editor.js

View workflow job for this annotation

GitHub Actions / build (20)

src/components/TaskPane/ActiveTaskDetails/ActiveTaskControls/TaskCompletionStep/TaskCompletionStep.test.jsx > TaskCompletionStep > renders task completion step 1 with required props

TypeError: Cannot read properties of undefined (reading 'zoom') ❯ constructIdURI src/services/Editor/Editor.js:207:47 ❯ Module.constructEditorUri src/services/Editor/Editor.js:756:14 ❯ UserEditorSelector.getEditorUri src/components/UserEditorSelector/UserEditorSelector.jsx:41:12 ❯ UserEditorSelector.render src/components/UserEditorSelector/UserEditorSelector.jsx:123:28 ❯ finishClassComponent node_modules/react-dom/cjs/react-dom.development.js:17485:31 ❯ updateClassComponent node_modules/react-dom/cjs/react-dom.development.js:17435:24 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:19073:16 ❯ HTMLUnknownElement.callCallback node_modules/react-dom/cjs/react-dom.development.js:3945:14 ❯ HTMLUnknownElement.callTheUserObjectsOperation node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30 ❯ innerInvokeEventListeners node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:350:25

Check failure on line 207 in src/services/Editor/Editor.js

View workflow job for this annotation

GitHub Actions / build (20)

src/components/TaskPane/ActiveTaskDetails/ActiveTaskControls/TaskCompletionStep/TaskCompletionStep.test.jsx > TaskCompletionStep > shows Edit button if allowedProgressions includes 1

TypeError: Cannot read properties of undefined (reading 'zoom') ❯ constructIdURI src/services/Editor/Editor.js:207:47 ❯ Module.constructEditorUri src/services/Editor/Editor.js:756:14 ❯ UserEditorSelector.getEditorUri src/components/UserEditorSelector/UserEditorSelector.jsx:41:12 ❯ UserEditorSelector.render src/components/UserEditorSelector/UserEditorSelector.jsx:123:28 ❯ finishClassComponent node_modules/react-dom/cjs/react-dom.development.js:17485:31 ❯ updateClassComponent node_modules/react-dom/cjs/react-dom.development.js:17435:24 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:19073:16 ❯ HTMLUnknownElement.callCallback node_modules/react-dom/cjs/react-dom.development.js:3945:14 ❯ HTMLUnknownElement.callTheUserObjectsOperation node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30 ❯ innerInvokeEventListeners node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:350:25

// iD only supports a single selected entity, so don't bother passing bundle
const selectedEntityComponent = osmObjectParams(task, false, "=", "&", options);
Expand Down Expand Up @@ -747,3 +737,40 @@
const matchingKey = _find(acceptableKeys, (key) => object[key]);
return matchingKey ? object[matchingKey] : undefined;
};

/**
* Constructs the appropriate editor URI based on the editor type
*/
export const constructEditorUri = (
editor,
task,
mapBounds,
options = {},
taskBundle = null,
comment = null,
) => {
if (!task) return null;

switch (editor) {
case Editor.id:
return constructIdURI(task, mapBounds, options, taskBundle, comment);
case Editor.level0:
return constructLevel0URI(task, mapBounds, options, taskBundle, comment);
case Editor.rapid:
return constructRapidURI(task, mapBounds, options, comment);
default:
return null;
Comment on lines +761 to +762
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this should throw; it's a programmer error to call this function with an editor ID other than one of the supported cases.

}
};

/**
* Gets the existing editor window reference
*/
export const getEditorWindowReference = () => editorWindowReference;

/**
* Sets the editor window reference
*/
export const setEditorWindowReference = (windowRef) => {
editorWindowReference = windowRef;
};
Loading