Skip to content

Update/unify link interfaces #6392 #7022

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

Closed
RockinRonE wants to merge 29 commits into from
Closed

Update/unify link interfaces #6392 #7022

Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
10e4082
Remove submit button; Add ellipsis button
RockinRonE May 27, 2018
7a99dfe
add toggle settings to state
RockinRonE May 27, 2018
3aaf90a
Add setLinkTarget func
RockinRonE May 27, 2018
14e09dc
Pass props to UrlInputButton
RockinRonE May 27, 2018
efdef98
Add set target attribute
RockinRonE May 27, 2018
2d65e82
Add target attribute
RockinRonE May 27, 2018
66d91f7
Add rel attribute
RockinRonE May 28, 2018
b604f7e
Set rel attribute
RockinRonE May 28, 2018
cd05ad6
Add prependHTTP
RockinRonE May 28, 2018
13025ab
Update blockAttributes with target and rel
RockinRonE May 28, 2018
b1aec98
Add popover and remove commented out code
RockinRonE May 29, 2018
2935e5f
Add onKeyDown functions
RockinRonE May 29, 2018
65541c4
Add keyDown handling
RockinRonE May 29, 2018
a79a991
Add setAttributes func to submit
RockinRonE May 29, 2018
3d9ce25
Add edit link section
RockinRonE May 29, 2018
141bf49
Remove box-shadow on focus
RockinRonE May 29, 2018
0b23bea
Add null default value
RockinRonE May 29, 2018
b538789
Add correct input and edit toggle forms
RockinRonE May 29, 2018
85f9fa3
Destructure props and remove toggle on pressing enter
RockinRonE May 29, 2018
f109c4c
Add toggle state reflected in toggle button
RockinRonE May 29, 2018
45193d2
Add disable lint and remove log statement
RockinRonE May 29, 2018
5d45dfb
Add href and target attrs to tests
RockinRonE May 29, 2018
4bc4620
Remove close button test
RockinRonE May 29, 2018
acb5b5a
Remove close form test
RockinRonE May 30, 2018
0edf482
fix merge conflicts
RockinRonE May 30, 2018
c14e4b3
fix formatting
RockinRonE May 30, 2018
345d3e7
fix formatting
RockinRonE May 30, 2018
ce4d81b
remove unused import
RockinRonE May 30, 2018
ccecb2b
remove whitespace
RockinRonE May 30, 2018
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
Prev Previous commit
Next Next commit
Add setLinkTarget func
  • Loading branch information
@RockinRonE
RockinRonE committed May 27, 2018
commit 3aaf90a326c995368854445b156ee229b81c2826
23 changes: 20 additions & 3 deletions editor/components/url-input/button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import classnames from 'classnames';
import './style.scss';
import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { IconButton } from '@wordpress/components';
import { IconButton, ToggleControl } from '@wordpress/components';

/**
* Internal dependencies
Expand All @@ -22,15 +22,17 @@ class UrlInputButton extends Component {
this.toggle = this.toggle.bind(this);
this.submitLink = this.submitLink.bind(this);
this.toggleLinkSettingsVisibility = this.toggleLinkSettingsVisibility.bind(this);
this.setLinkTarget = this.setLinkTarget.bind(this);

this.state = {
expanded: false,
settingsVisible: false,
opensInNewWindow: false,
};
}

// this.state = {
// opensInNewWindow: false,
//
// linkValue: '',
// };

Expand All @@ -47,11 +49,25 @@ class UrlInputButton extends Component {
this.setState((state) => ({ settingsVisible: !state.settingsVisible }));
}

setLinkTarget(opensInNewWindow) {
this.setState({ opensInNewWindow });
}

render() {
const { url, onChange } = this.props;
const { expanded } = this.state;
const { expanded, settingsVisible, opensInNewWindow } = this.state;
const buttonLabel = url ? __('Edit Link') : __('Insert Link');

const linkSettings = settingsVisible && (
<div className="editor-format-toolbar__link-modal-line editor-format-toolbar__link-settings">
Copy link
Member

@aduth aduth May 31, 2018

Choose a reason for hiding this comment

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

The coding guidelines for CSS naming is written such that this is not a valid class name to assign to this component. It's reflective of the fact that components are meant to compose one another, and shouldn't be inheriting behaviors, since it can lead to accidental breakage if a change to the format toolbar inadvertently affects this component. Isolating components to silo their behavior / silos helps prevent this.

What I'd like to see instead, and what seemed to be hinted by the idea of "unifying" in the original pull request, is that instead of duplicating what already exists elsewhere, we ought to consider creating a new, separate component which contains the shared behavior (of link management), then use that both here and in other existing usage.

<ToggleControl
label={__('Open in new window')}
checked={opensInNewWindow}
onChange={this.setLinkTarget}
/>
</div>
);

return (
<div className="editor-url-input__button">
<IconButton
Expand Down Expand Up @@ -82,6 +98,7 @@ class UrlInputButton extends Component {
// aria-expanded={settingsVisible}
/>
</div>
{linkSettings}
</form>
}
</div>
Expand Down