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

Jump to bottom

HelpTrigger #3431

Merged
merged 2 commits into from
Feb 13, 2019
Merged

HelpTrigger #3431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fb77ca
Moved to HelpTrigger
ranbena Feb 13, 2019
b29ffe6
Moved share dialog “Learn more” to HelpTrigger
ranbena Feb 13, 2019
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
15 changes: 2 additions & 13 deletions client/app/components/ParameterMappingInput.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,12 @@ import { ParameterValueInput } from '@/components/ParameterValueInput';
import { ParameterMappingType } from '@/services/widget';
import { clientConfig } from '@/services/auth';
import { Query, Parameter } from '@/services/query';
import HelpTrigger from '@/services/HelpTrigger';

import './ParameterMappingInput.less';

const { Option } = Select;

const HELP_URL = [
'https://redash.io/help/user-guide/querying/query-parameters?source=dialog#Value-Source-Options',
'Guide: Value Source Options',
];

export const MappingType = {
DashboardAddNew: 'dashboard-add-new',
DashboardMapToExisting: 'dashboard-map-to-existing',
Expand Down Expand Up @@ -339,18 +335,11 @@ class MappingEditor extends React.Component {

renderContent() {
const { mapping, inputError } = this.state;
const [helpUrl, tooltip] = HELP_URL;

return (
<div className="parameter-mapping-editor">
<header>
Edit Source and Value
{/* eslint-disable-next-line react/jsx-no-target-blank */}
<a href={helpUrl} target="_blank" rel="noopener">
<Tooltip title={tooltip}>
<Icon type="question-circle" />
</Tooltip>
</a>
Edit Source and Value <HelpTrigger type="VALUE_SOURCE_OPTIONS" />
Copy link
Member

Choose a reason for hiding this comment

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

Why not pass the title and URL here? I'm pretty sure we will rarely reuse the link helps, so having them in a single place and then referenced using some random string is just recipe for trouble.

Copy link
Contributor Author

@ranbena ranbena Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not meant for reuse, but so that if help urls change, there's a single source to edit.
This is a "preparation for A/C" for when TS is integrated as it would enforce the string to be of type (I don't like unnecessary type importing..).

👍 if you're cool with it, 👎 if you'd rather go with your suggestion

Copy link
Member

Choose a reason for hiding this comment

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

You mean that TS will enforce that the string is one of possible values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

Copy link
Member

Choose a reason for hiding this comment

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

TS support can't come any sooner... 😓

I wonder if we should go through the effort of introducing it now, or wait to get rid of Angular and then switch to react-scripts and let them take care of the plumbing...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really dunno to tell you the amount of effort involved.
If it's truly possible to gradually adopt - let's do this now.
If not - it can wait.

The focus for the coming 4 weeks should be Angular->React migration completion.

</header>
<ParameterMappingInput
mapping={mapping}
Expand Down
8 changes: 2 additions & 6 deletions client/app/pages/dashboards/ShareDashboardDialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ import PropTypes from 'prop-types';
import Switch from 'antd/lib/switch';
import Modal from 'antd/lib/modal';
import Form from 'antd/lib/form';
import Tooltip from 'antd/lib/tooltip';
import { $http, toastr } from '@/services/ng';
import { wrap as wrapDialog, DialogPropType } from '@/components/DialogWrapper';
import InputWithCopy from '@/components/InputWithCopy';
import HelpTrigger from '@/services/HelpTrigger';

const API_SHARE_URL = 'api/dashboards/{id}/share';
const HELP_URL = 'https://redash.io/help/user-guide/dashboards/sharing-dashboards?source=dialog';

class ShareDashboardDialog extends React.Component {
static propTypes = {
Expand Down Expand Up @@ -38,10 +37,7 @@ class ShareDashboardDialog extends React.Component {
Share Dashboard
<div className="modal-header-desc">
Allow public access to this dashboard with a secret address.{' '}
<Tooltip title="Guide: Sharing and Embedding Dashboards">
{ /* eslint-disable-next-line react/jsx-no-target-blank */}
<a href={HELP_URL} target="_blank" rel="noopener">Learn more</a>
</Tooltip>
<HelpTrigger type="SHARE_DASHBOARD" />
</div>
</React.Fragment>
);
Expand Down
35 changes: 35 additions & 0 deletions client/app/services/HelpTrigger.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import React from 'react';
import PropTypes from 'prop-types';
import Tooltip from 'antd/lib/tooltip';
import Icon from 'antd/lib/icon';

const BASE_URL = 'https://redash.io/help/user-guide/';
const TYPES = {
VALUE_SOURCE_OPTIONS: [
'querying/query-parameters#Value-Source-Options',
'Value Source Options',
],
SHARE_DASHBOARD: [
'dashboards/sharing-dashboards',
'Sharing and Embedding Dashboards',
],
};

export default class HelpTrigger extends React.PureComponent {
static propTypes = {
type: PropTypes.oneOf(Object.keys(TYPES)).isRequired,
}

render() {
const [path, tooltip] = TYPES[this.props.type];
const href = BASE_URL + path;
return (
<Tooltip title={`Guide: ${tooltip}`}>
{/* eslint-disable-next-line react/jsx-no-target-blank */}
<a href={href} target="_blank" rel="noopener">
<Icon type="question-circle" />
</a>
</Tooltip>
);
}
}