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

MNT Remove TODO comments #1608

Merged
Merged
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
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion client/src/boot/BootRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ class BootRoutes {

// Check if the beginning of the route is the same as the current location.
// Since we haven't decided on a router yet, we can't use it for route matching.
// TODO Limit full page load when transitioning from legacy to react route or vice versa
return currentPath.match(route);
});
}
Expand Down
1 change: 0 additions & 1 deletion client/src/boot/applyTransforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const applyTransforms = () => {
Injector.transform(
'field-holders',
(updater) => {
// @todo: Should contain every field that exports itself wrapped in a `fieldHolder` by default
const fields = [
'FieldGroup',
];
Expand Down
1 change: 0 additions & 1 deletion client/src/boot/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ async function appBoot() {
routes.setStore(store);
routes.start(window.location.pathname);

// @todo - Remove once we remove entwine
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
// Enable top-level css selectors for react-dependant entwine sections
if (window.jQuery) {
// need to separate class adds ...because entwine...
Expand Down
1 change: 0 additions & 1 deletion client/src/components/Badge/Badge.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
letter-spacing: .3px;
}

// Todo: replace .status-archived class name with .badge--[modifier]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #1595

.status-archived {
color: $text-muted;
}
Expand Down
4 changes: 2 additions & 2 deletions client/src/components/Breadcrumb/Breadcrumb.scss
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
}
}

.breadcrumb > li.breadcrumb__item--last, // TODO Fix Bootstrap clash
.breadcrumb > li.breadcrumb__item--last,
.breadcrumb__item--last {
display: block;
float: none;
Expand All @@ -71,7 +71,7 @@
}
}

.cms h2.breadcrumb__item-title--last, // TODO Fix CMS clash
.cms h2.breadcrumb__item-title--last,
.breadcrumb__item--last .breadcrumb__item-title,
.breadcrumb__item-title--last {
margin: 0;
Expand Down
4 changes: 0 additions & 4 deletions client/src/components/Form/Form.scss
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,13 @@
}

// Reset .file bootstrap overrides to default styles
// TODO rename .file to a different class name as it conflicts with default bootstrap
&.file {
height: auto;
display: block;
cursor: auto;
position: static;
}

// TODO Fix for when the .form-group--no-label class is used but there is actually a label
&.form-group--no-label:not(.stacked) .form__field-label + .form__field-holder {
margin-left: 0;
}
Expand Down Expand Up @@ -213,7 +211,6 @@ input.radio {
@include make-row();

// Composite fields
// TODO reduce nesting
.form__field-holder .form-group {
.form__field-holder,
.form__field-label {
Expand All @@ -231,7 +228,6 @@ input.radio {
}
}

// TODO make label display on the right side like normal .form__field-extra-label
.form__field-extra-label {
@include make-col(12);
margin-left: 0;
Expand Down
10 changes: 2 additions & 8 deletions client/src/components/FormAction/FormAction.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// TODO Separate out bootstrap btn reset styles to a separate style sheet or divide within this sheet
// TODO Rename component to something like Btn or Button?

// Button wrapper
.btn-toolbar {
margin-top: $spacer;
Expand Down Expand Up @@ -65,8 +62,7 @@
font-size: 17px;
}

// Please use .btn--icon-lg
// TODO .btn--icon-large to be deprecated across CMS
// Please use .btn--icon-lg
.btn--icon-large[class*="font-icon-"]::before,
.btn--icon-lg[class*="font-icon-"]::before {
font-size: 20px;
Expand All @@ -77,7 +73,6 @@
}

// For buttons with icon and no text, removes space after icon
// TODO replace all .no-text classes for .btn--no-text
.btn--no-text[class*="font-icon-"]::before,
.no-text[class*="font-icon-"]::before {
margin-right: 0;
Expand Down Expand Up @@ -180,7 +175,6 @@
}
}

// Todo: All secondary buttons need to change to use .btn-light
.btn-secondary {
border-color: transparent;
background-color: transparent;
Expand Down Expand Up @@ -255,7 +249,7 @@
}
}

// For secondary type actions without border, TODO change word "outline" to border
// For secondary type actions without border
.btn-hide-outline {
border-color: transparent;
}
Expand Down
1 change: 0 additions & 1 deletion client/src/components/FormAlert/FormAlert.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ class FormAlert extends Component {
}

render() {
// @todo default this.props.visible as null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #1595

if ((typeof this.props.visible !== 'boolean' && this.state.visible) || this.props.visible) {
// needs to be inside a div because the `Alert` component does some magic with props.children
const body = castStringToElement('div', this.props.value);
Expand Down
1 change: 0 additions & 1 deletion client/src/components/FormBuilder/FormBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ class FormBuilder extends Component {
return formSchema;
})
.catch((reason) => {
// @todo Generic CMS error reporting
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #1595

this.setState({ submittingAction: null });
throw reason;
});
Expand Down
5 changes: 0 additions & 5 deletions client/src/components/GridField/GridField.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ const NotYetLoaded = [];
/**
* The component acts as a container for a grid field,
* with smarts around data retrieval from external sources.
*
* @todo Convert to higher order component which hooks up form
* schema data to an API backend as a grid data source
* @todo Replace "dumb" inner components with third party library (e.g. https://griddlegriddle.github.io)
*/
class GridField extends Component {
constructor(props) {
Expand Down Expand Up @@ -134,7 +130,6 @@ class GridField extends Component {

render() {
if (this.props.records === NotYetLoaded) {
// TODO Replace with better loading indicator
return <div>{ i18n._t('CampaignAdmin.LOADING', 'Loading...') }</div>;
}

Expand Down
14 changes: 6 additions & 8 deletions client/src/components/GridField/GridField.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Grid-field
// Uses bootstrap .table styles
// TODO remove nesting once buttons have been BEMified

.grid-field {
border-bottom: 0;
Expand Down Expand Up @@ -67,7 +66,7 @@
font-family: silverstripe;
}

&.ss-gridfield-sorted-desc, // TODO BEMify class
&.ss-gridfield-sorted-desc,
&.ss-gridfield-sorted-asc {
border-bottom: $table-border-width solid $cyan;

Expand All @@ -77,11 +76,11 @@
}
}

&.ss-gridfield-sorted-desc::after { // TODO BEMify classes
&.ss-gridfield-sorted-desc::after {
content: "*";
}

&.ss-gridfield-sorted-asc::after { // TODO BEMify classes
&.ss-gridfield-sorted-asc::after {
content: "(";
}

Expand Down Expand Up @@ -147,7 +146,7 @@ input.grid-field__sort-field {
padding-right: 30px;
}

.grid-field & { // TODO Reduce nesting
.grid-field & {
width: calc(100% + #{$input-btn-padding-x * 2});
border-color: $table-bg-tools;
}
Expand Down Expand Up @@ -178,7 +177,7 @@ div.grid-field__sort-field + .form__fieldgroup-item {

// Grid-field body actions
.grid-field__icon-action,
.grid-field__icon-action[class*="font-icon-"] { // Override legacy styles (TODO: remove once jqueryui is removed)
.grid-field__icon-action[class*="font-icon-"] { // Override legacy styles
background: none;
border: 0;
padding: $table-cell-padding calc($table-cell-padding / 2);
Expand Down Expand Up @@ -230,7 +229,7 @@ div.grid-field__sort-field + .form__fieldgroup-item {
padding: 16px 20px;
display: block;

.grid-field .grid-field__table & { // Override legacy styles (TODO: remove once jqueryui is removed)
.grid-field .grid-field__table & { // Override legacy styles
color: $text-muted;
text-decoration: none;
}
Expand All @@ -255,7 +254,6 @@ div.grid-field__sort-field + .form__fieldgroup-item {


// Responsive grid-field
// Todo: add .text-truncate for overflowing cells
@include media-breakpoint-down(sm) {
.grid-field__table td,
.grid-field__table th {
Expand Down
9 changes: 0 additions & 9 deletions client/src/components/GridField/GridFieldTable.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

opent ticket: #1597

Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ class GridFieldTable extends Component {
return this.props.header;
}

if (typeof this.props.data !== 'undefined') {
// TODO: Generate the header.
}

return null;
}

Expand All @@ -35,10 +31,6 @@ class GridFieldTable extends Component {
return this.props.rows;
}

if (typeof this.props.data !== 'undefined') {
// TODO: Generate the rows.
}

return null;
}

Expand All @@ -55,7 +47,6 @@ class GridFieldTable extends Component {
}

GridFieldTable.propTypes = {
data: PropTypes.object,
header: PropTypes.object,
rows: PropTypes.array,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class SingleSelectField extends Component {
*/
getInputProps() {
const props = {
// @TODO Prevent entwine chosen applying chosen
className: `${this.props.className} ${this.props.extraClass} no-chosen`,
id: this.props.id,
name: this.props.name,
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/Tabs/Tabs.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Tabs, styles built on top of Bootstrap 4 tab functionality
.nav-tabs {
margin-bottom: $panel-padding-y;
border-radius: 0; // TODO remove once JQueryUI is removed
border-radius: 0;

// Spacing between items
.nav-item + .nav-item {
Expand Down
1 change: 0 additions & 1 deletion client/src/components/Toolbar/Toolbar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ $circular-button-border-radius: 100px;
$circular-button-size: 32px;
$circular-button-padding: 7px;

// TODO ensure .toolbar is used globally and remove toolbar--"modifier" classes from following block
.toolbar,
.toolbar--north,
.toolbar--content,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ class TreeDropdownField extends Component {
// If any ancestor node in visible chain is either loading or failed then abort re-load
const foundPrev = path.find((pathNode) => (
this.props.loading.indexOf(pathNode) > -1
// TODO: investigate whether failed should not retry
|| this.props.failed.indexOf(pathNode) > -1
));
if (foundPrev) {
Expand Down
3 changes: 0 additions & 3 deletions client/src/containers/FormBuilderLoader/FormBuilderLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ class FormBuilderLoader extends Component {
const messages = {};

// only error messages are collected
// TODO define message type as standard "success", "info", "warning" and "danger"
if (state && state.fields) {
state.fields.forEach((field) => {
if (field.message) {
Expand Down Expand Up @@ -132,8 +131,6 @@ class FormBuilderLoader extends Component {
}

return promise
// TODO Suggest storing messages in a separate redux store rather than throw an error
// ref: https://github.com/erikras/redux-form/issues/94#issuecomment-143398399
.then(formSchema => {
if (!formSchema || !formSchema.state) {
return formSchema;
Expand Down
3 changes: 0 additions & 3 deletions client/src/legacy/ConfirmedPasswordField.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import $ from 'jquery';

// TODO Enable once https://github.com/webpack/extract-text-webpack-plugin/issues/179 is resolved. Included in bundle.scss for now.
// import '../styles/legacy/ConfirmedPasswordField.scss';

$(document).on('click', '.confirmedpassword .showOnClick a', function () {
var $container = $('.showOnClickContainer', $(this).parent());

Expand Down
1 change: 0 additions & 1 deletion client/src/legacy/DateField.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ jQuery.entwine('ss', ($) => {
this.updateValue();
},
onchange() {
// TODO Validation
this.updateValue();
},
updateValue() {
Expand Down
1 change: 0 additions & 1 deletion client/src/legacy/DatetimeField.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ jQuery.entwine('ss', ($) => {
this.updateValue();
},
onchange() {
// TODO Validation
this.updateValue();
},
updateValue() {
Expand Down
9 changes: 1 addition & 8 deletions client/src/legacy/GridField.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import { loadComponent } from 'lib/Injector';
import '../../../thirdparty/jquery-ui/jquery-ui.js';
import '../../../thirdparty/jquery-entwine/jquery.entwine.js';

// TODO Enable once https://github.com/webpack/extract-text-webpack-plugin/issues/179 is resolved. Included in bundle.scss for now.
// import '../styles/legacy/GridField.scss';

$.entwine('ss', function($) {
$('.grid-field').entwine({
onmatch: function () {
Expand Down Expand Up @@ -91,8 +88,7 @@ $.entwine('ss', function($) {
dataType: 'html',
success: function (data) {
// Replace the grid field with response, not the form.
// TODO Only replaces all its children, to avoid replacing the current scope
// of the executing method. Means that it doesn't retrigger the onmatch() on the main container.
// It doesn't retrigger the onmatch() on the main container.
self.empty().append($(data).children());

// Refocus previously focused element. Useful e.g. for finding+adding
Expand Down Expand Up @@ -323,7 +319,6 @@ $.entwine('ss', function($) {

const GridFieldActions = this.getComponent();

// TODO: rework entwine so that react has control of holder
let root = this.getReactRoot();
if (!root) {
root = createRoot(this[0]);
Expand Down Expand Up @@ -651,8 +646,6 @@ $.entwine('ss', function($) {
$('.grid-field[data-selectable] .ss-gridfield-items').entwine({
onadd: function() {
this._super();

// TODO Limit to single selection
this.selectable();
},
onremove: function() {
Expand Down
1 change: 0 additions & 1 deletion client/src/legacy/LeftAndMain.BatchActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ $.entwine('ss.tree', function($){
}
}

// TODO Should work by triggering change() along, but doesn't - entwine event bubbling?
this.trigger('chosen:updated');

this._super(e);
Expand Down
9 changes: 0 additions & 9 deletions client/src/legacy/LeftAndMain.EditForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ $.entwine('ss', function($){
}
}

// TODO
// // Rewrite # links
// html = html.replace(/(<a[^>]+href *= *")#/g, '$1' + window.location.href.replace(/#.*$/,'') + '#');
//
// // Rewrite iframe links (for IE)
// html = html.replace(/(<iframe[^>]*src=")([^"]+)("[^>]*>)/g, '$1' + $('base').attr('href') + '$2$3');

GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
this._super();
},
'from .cms-tabset': {
Expand Down Expand Up @@ -282,8 +275,6 @@ $.entwine('ss', function($){
* Hook in (optional) validation routines.
* Currently clientside validation is not supported out of the box in the CMS.
*
* Todo:
* Placeholder implementation
*
* Returns:
* {boolean}
Expand Down
Loading