chore(many): add tech debt tickets and remove unnecessary todos (#26266)

This commit is contained in:
Amanda Johnston
2022-11-14 14:49:08 -06:00
committed by GitHub
parent 5a701b5b42
commit e3ef932ae1
40 changed files with 17 additions and 168 deletions

View File

@@ -83,14 +83,8 @@ export class ValueAccessor implements ControlValueAccessor, AfterViewInit, OnDes
}
/**
* TODO Remove this in favor of https://github.com/angular/angular/issues/10887
* whenever it is implemented. Currently, Ionic's form status classes
* do not react to changes when developers manually call
* Angular form control methods such as markAsTouched.
* This results in Ionic's form status classes being out
* of sync with the ng form status classes.
* This patches the methods to manually sync
* the classes until this feature is implemented in Angular.
* TODO FW-2787: Remove this in favor of https://github.com/angular/angular/issues/10887
* whenever it is implemented.
*/
const formControl = ngControl.control as any;
if (formControl) {

View File

@@ -74,7 +74,6 @@ export const toSegments = (path: string): string[] => {
export const destroyView = (view: RouteView | undefined): void => {
if (view) {
// TODO lifecycle event
view.ref.destroy();
view.unlistenEvents();
}

View File

@@ -11,7 +11,6 @@ export class OverlayBaseController<Opts, Overlay> implements ControllerShape<Opt
* Creates a new overlay
*/
create(opts?: Opts): Promise<Overlay> {
// TODO: next major release opts is not optional
return this.ctrl.create((opts || {}) as any);
}

View File

@@ -73,7 +73,7 @@ Cypress.Commands.add('ionPageDoesNotExist', (selector) => {
});
Cypress.Commands.add('ionTabClick', (tabText) => {
// TODO: Figure out how to get rid of wait. It's a workaround for flakiness in CI.
// TODO FW-2790: Figure out how to get rid of wait. It's a workaround for flakiness in CI.
cy.wait(250);
cy.contains('ion-tab-button', tabText).click({ force: true });
});

View File

@@ -4,7 +4,7 @@ import { test } from '@utils/test/playwright';
test.describe('accordion: a11y', () => {
test('accordions should be keyboard navigable', async ({ page, skip, browserName }) => {
// TODO(FW-1764): remove skip once issue is resolved
skip.browser('firefox', 'https://github.com/ionic-team/ionic-framework/issues/25529');
skip.browser('firefox', 'https://github.com/ionic-team/ionic-framework/issues/25070');
await page.goto(`/src/components/accordion/test/a11y`);
const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';

View File

@@ -25,7 +25,6 @@
// iOS Checkbox: Disabled
// -----------------------------------------
// TODO: .item-ios.item-checkbox-disabled ion-label
:host(.checkbox-disabled) {
opacity: $checkbox-ios-disabled-opacity;
}

View File

@@ -44,7 +44,6 @@
// Material Design Checkbox: Disabled
// --------------------------------------------------------
// TODO .item-md.item-checkbox-disabled ion-label
:host(.checkbox-disabled) {
opacity: $checkbox-md-disabled-opacity;
}

View File

@@ -54,7 +54,6 @@
color: current-color(contrast);
}
// TODO we should remove outer-content in favor of a color
:host(.outer-content) {
--background: #{$background-color-step-50};
}

View File

@@ -280,7 +280,6 @@ export class Content implements ComponentInterface {
if (easedT < 1) {
// do not use DomController here
// must use nativeRaf in order to fire in the next frame
// TODO: remove as any
requestAnimationFrame(step);
} else {
resolve();

View File

@@ -1038,7 +1038,7 @@ export class Datetime implements ComponentInterface {
this.initializeListeners();
/**
* TODO: Datetime needs a frame to ensure that it
* TODO FW-2793: Datetime needs a frame to ensure that it
* can properly scroll contents into view. As a result
* we hide the scrollable content until after that frame
* so users do not see the content quickly shifting. The downside

View File

@@ -17,7 +17,7 @@ describe('getPartsFromCalendarDay()', () => {
});
});
// TODO: parseDate()
// TODO FW-2794: parseDate()
describe('clampDate()', () => {
const minParts = {

View File

@@ -183,15 +183,6 @@
}
// TODO: MOVE FROM RADIO
// iOS Radio Item Label: Checked
// -----------------------------------------
// :host(.item-radio-checked) ::slotted(ion-label) {
// color: $radio-ios-color-on;
// }
// iOS Slotted Label
// --------------------------------------------------
@@ -208,7 +199,6 @@
--min-height: 68px;
}
// TODO: refactor, ion-item and ion-textarea have the same CSS
:host(.item-label-stacked) ::slotted(ion-select),
:host(.item-label-floating) ::slotted(ion-select) {
--padding-top: 8px;
@@ -224,32 +214,3 @@
:host(.item-label-fixed) ::slotted(ion-datetime) {
--padding-start: 0;
}
// FROM TEXTAREA
// iOS Stacked & Floating Textarea
// --------------------------------------------------
// TODO
// .item-ios.item-label-stacked .label-ios + .input + .cloned-input,
// .item-ios.item-label-floating .label-ios + .input + .cloned-input {
// @include margin-horizontal(0, null);
// }
// iOS Input After Label
// --------------------------------------------------
// .label-ios + .input .native-input,
// .label-ios + .input + .cloned-input {
// @include margin-horizontal($input-ios-by-label-margin-start, null);
// }
// iOS Textarea After Label
// --------------------------------------------------
// .label-ios + ion-textarea .native-textarea,
// .label-ios + .input + .cloned-input {
// @include margin-horizontal($textarea-ios-by-label-margin-start, null);
// width: calc(100% - (#{$item-ios-padding-end} / 2) - #{$item-ios-padding-start});
// }

View File

@@ -168,7 +168,6 @@
// Material Design Multi-line Item
// --------------------------------------------------
// TODO this works if manually adding the class / should it work with prop?
// Multi-line items should align the slotted content at the top
:host(.item-multi-line) ::slotted([slot="start"]),
:host(.item-multi-line) ::slotted([slot="end"]) {
@@ -362,7 +361,6 @@
--min-height: 55px;
}
// TODO: refactor, ion-item and ion-textarea have the same CSS
:host(.item-label-stacked) ::slotted(ion-select),
:host(.item-label-floating) ::slotted(ion-select) {
--padding-top: 8px;

View File

@@ -37,7 +37,7 @@ export class Menu implements ComponentInterface, MenuI {
private blocker = GESTURE_CONTROLLER.createBlocker({ disableScroll: true });
isAnimating = false;
width!: number; // TODO
width!: number;
_isOpen = false;
backdropEl?: HTMLElement;
@@ -465,7 +465,6 @@ export class Menu implements ComponentInterface, MenuI {
}
if (this._isOpen) {
return true;
// TODO error
} else if (menuController._getOpenSync()) {
return false;
}
@@ -534,10 +533,9 @@ export class Menu implements ComponentInterface, MenuI {
let newStepValue = shouldComplete ? 0.001 : -0.001;
/**
* TODO: stepValue can sometimes return a negative
* stepValue can sometimes return a negative
* value, but you can't have a negative time value
* for the cubic bezier curve (at least with web animations)
* Not sure if the negative step value is an error or not
*/
const adjustedStepValue = stepValue < 0 ? 0.01 : stepValue;

View File

@@ -81,13 +81,6 @@ export const convertToViews = (pages: NavComponentWithProps[]): ViewController[]
return page;
}
if ('component' in page) {
/**
* TODO Ionic 6:
* Consider switching to just using `undefined` here
* as well as on the public interfaces and on
* `NavComponentWithProps`. Previously `pages` was
* of type `any[]` so TypeScript did not catch this.
*/
return convertToView(page.component, page.componentProps === null ? undefined : page.componentProps);
}
return convertToView(page, undefined);

View File

@@ -239,8 +239,6 @@ export class PickerColumnCmp implements ComponentInterface {
return Math.min(Math.max(Math.abs(Math.round(y / this.optHeight)), 0), this.col.options.length - 1);
}
// TODO should this check disabled?
private onStart(detail: GestureDetail) {
// We have to prevent default in order to block scrolling under the picker
// but we DO NOT have to stop propagation, since we still want

View File

@@ -208,10 +208,6 @@
var radioValues = ['fruitRadio', 'pizzaRadio', 'veggiesRadio'];
printRadioValues();
function printForm(ev) {
console.log('TODO get working with forms');
}
function printRadioValues() {
var html = 'Values:<br>';

View File

@@ -714,8 +714,6 @@ export class Refresher implements ComponentInterface {
// set that the refresh is actively cancelling/completing
this.state = state;
this.setCss(0, this.closeDuration, true, delay);
// TODO: stop gesture
}
private setCss(y: number, duration: string, overflowVisible: boolean, delay: string) {

View File

@@ -3,7 +3,7 @@ import { test } from '@utils/test/playwright';
import { pullToRefresh } from '../test.utils';
// TODO: Enable this test when touch events/gestures are better supported in Playwright: https://github.com/microsoft/playwright/issues/2903
// TODO FW-2795: Enable this test when touch events/gestures are better supported in Playwright
test.skip('refresher: basic', () => {
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/refresher/test/basic');

View File

@@ -3,7 +3,7 @@ import { test } from '@utils/test/playwright';
import { pullToRefresh } from '../test.utils';
// TODO: Enable this test when touch events/gestures are better supported in Playwright: https://github.com/microsoft/playwright/issues/2903
// TODO FW-2795: Enable this test when touch events/gestures are better supported in Playwright
test.skip('refresher: custom scroll target', () => {
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/refresher/test/scroll-target');

View File

@@ -245,9 +245,6 @@ export class ReorderGroup implements ComponentInterface {
private itemIndexForTop(deltaY: number): number {
const heights = this.cachedHeights;
// TODO: since heights is a sorted array of integers, we can do
// speed up the search using binary search. Remember that linear-search is still
// faster than binary-search for small arrays (<64) due CPU branch misprediction.
for (let i = 0; i < heights.length; i++) {
if (heights[i] > deltaY) {
return i;

View File

@@ -493,7 +493,6 @@ export class Select implements ComponentInterface {
* Close the select interface.
*/
private close(): Promise<boolean> {
// TODO check !this.overlay || !this.isFocus()
if (!this.overlay) {
return Promise.resolve(false);
}

View File

@@ -353,7 +353,6 @@ export class Slides implements ComponentInterface {
private normalizeOptions(): SwiperOptions {
// Base options, can be changed
// TODO Add interface SwiperOptions
const swiperOptions: SwiperOptions = {
effect: undefined,
direction: 'horizontal',

View File

@@ -73,17 +73,6 @@
opacity: $toggle-md-disabled-opacity;
}
// TODO: move to item
// .item-md.item-toggle-disabled ion-label {
// opacity: $toggle-md-disabled-opacity;
// pointer-events: none;
// }
// .toggle-md.toggle-disabled ion-radio {
// opacity: $toggle-md-disabled-opacity;
// }
// Material Design Toggle Within An Item
// ----------------------------------------------------------

View File

@@ -25,11 +25,6 @@ body.backdrop-no-scroll {
overflow: hidden;
}
// TODO: Block scrolling in ion-content, breaks inside ion-modal
// body.backdrop-no-scroll .ion-page > ion-content {
// --overflow: hidden;
// }
// Modal - Card Style
// --------------------------------------------------
/**
@@ -253,7 +248,6 @@ ion-card-header.ion-color .ion-inherit-color {
// Menu Styles
// TODO: Find a better long term solution for this
// --------------------------------------------------
.menu-content {

View File

@@ -908,8 +908,6 @@ export const createAnimation = (animationId?: string): Animation => {
* may be flickering if a new
* animation is started on the same
* element too quickly
*
* TODO: Is there a cleaner way to do this?
*/
raf(() => {
clearCSSAnimationPlayState();

View File

@@ -2,7 +2,6 @@
* Based on:
* https://stackoverflow.com/questions/7348009/y-coordinate-for-a-given-x-cubic-bezier
* https://math.stackexchange.com/questions/26846/is-there-an-explicit-form-for-cubic-b%C3%A9zier-curves
* TODO: Reduce rounding error
*/
/**

View File

@@ -1,6 +1,3 @@
// import { RouterDirection } from '../interface';
// TODO router direction
// The interfaces in this file are used to make sure our components
// have the correct properties defined that are needed to pass to
// the native HTML elements they render
@@ -10,11 +7,9 @@ export interface AnchorInterface {
target: string | undefined;
rel: string | undefined;
download: string | undefined;
// routerDirection: RouterDirection;
}
export interface ButtonInterface {
type: 'submit' | 'reset' | 'button';
disabled: boolean;
// routerDirection: RouterDirection;
}

View File

@@ -66,11 +66,6 @@ export const createSwipeBackGesture = (
realDur = Math.min(dur, 540);
}
/**
* TODO: stepValue can sometimes return negative values
* or values greater than 1 which should not be possible.
* Need to investigate more to find where the issue is.
*/
onEndHandler(shouldComplete, stepValue <= 0 ? 0.01 : clamp(0, stepValue, 0.9999), realDur);
};

View File

@@ -41,7 +41,7 @@ export const enableInputBlurring = () => {
}
focused = false;
// TODO: find a better way, why 50ms?
// TODO FW-2796: find a better way, why 50ms?
setTimeout(() => {
if (!focused) {
active.blur();

View File

@@ -209,42 +209,6 @@ export const config: Config = {
'ion-picker-column',
'ion-virtual-scroll'
],
/**
* TODO: Abstract custom Ionic value accessor functionality
* to be configurable with Stencil generated value accessors.
*/
// valueAccessorConfigs: [
// {
// elementSelectors: ['ion-input:not([type=number])', 'ion-textarea', 'ion-searchbar'],
// event: 'ionChange',
// targetAttr: 'value',
// type: 'text',
// },
// {
// elementSelectors: ['ion-input[type=number]'],
// event: 'ionChange',
// targetAttr: 'value',
// type: 'number',
// },
// {
// elementSelectors: ['ion-checkbox', 'ion-toggle'],
// event: 'ionChange',
// targetAttr: 'checked',
// type: 'boolean',
// },
// {
// elementSelectors: ['ion-range', 'ion-select', 'ion-radio-group', 'ion-segment', 'ion-datetime'],
// event: 'ionChange',
// targetAttr: 'value',
// type: 'select',
// },
// {
// elementSelectors: ['ion-radio'],
// event: 'ionSelect',
// targetAttr: 'checked',
// type: 'radio',
// },
// ]
}),
],
buildEs5: 'prod',

View File

@@ -116,7 +116,7 @@ class IonRouterInner extends React.PureComponent<IonRouteProps, IonRouteState> {
this.incomingRouteParams = {
routeAction: 'replace',
routeDirection: 'none',
tab: this.currentTab, // TODO this isn't legit if replacing to a page that is not in the tabs
tab: this.currentTab,
};
}
if (action === 'POP') {

View File

@@ -133,7 +133,6 @@ describe('Routing Tests', () => {
it('Tab 3 > Other Page > Back, should be back on Tab 3', () => {
// Tests transferring from one outlet to another and back again with animation
// TODO: how to test the transition animation happens?
cy.visit(`http://localhost:${port}/routing/tabs/tab3`);
cy.ionNav('ion-button', 'Go to Other Page');
cy.ionPageVisible('other-page');

View File

@@ -101,7 +101,7 @@ Cypress.Commands.add('ionMenuNav', (contains) => {
});
Cypress.Commands.add('ionTabClick', (tabText) => {
// TODO: figure out how to get rid of this wait. Switching tabs after a forward nav to a details page needs it
// TODO FW-2800: figure out how to get rid of this wait. Switching tabs after a forward nav to a details page needs it
cy.wait(500);
cy.contains('ion-tab-button', tabText).click({ force: true });
// cy.get('ion-tab-button.tab-selected').contains(tabText)

View File

@@ -165,7 +165,7 @@ export const setupIonicReact = (config: IonicConfig = {}) => {
* By default Ionic Framework hides elements that
* are not hydrated, but in the CE build there is no
* hydration.
* TODO: Remove when all integrations have been
* TODO FW-2797: Remove when all integrations have been
* migrated to CE build.
*/
if (typeof (document as any) !== 'undefined') {

View File

@@ -66,7 +66,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
let currentRouteInfo: RouteInfo;
let incomingRouteParams: RouteParams;
// TODO types
let historyChangeListeners: any[] = [];
if (typeof (document as any) !== 'undefined') {
@@ -104,7 +103,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
});
const handleNavigateBack = (defaultHref?: string, routerAnimation?: AnimationBuilder) => {
// todo grab default back button href from config
const routeInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition);
if (routeInfo && routeInfo.pushedByRoute) {
const prevInfo = locationHistory.findLastLocation(routeInfo);
@@ -561,7 +559,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
}
}
// TODO types
const registerHistoryChangeListener = (cb: any) => {
historyChangeListeners.push(cb);
}

View File

@@ -63,7 +63,7 @@ export interface ViewItem {
outletId: number;
matchedRoute: RouteLocationMatched;
ionPageElement?: HTMLElement;
vueComponent: any; // todo
vueComponent: any;
ionRoute: boolean;
mount: boolean;
exact: boolean;

View File

@@ -39,7 +39,6 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({
const ionRouterOutlet = ref();
const id = generateId('ion-router-outlet');
// TODO types
const ionRouter: any = inject('navManager');
const viewStacks: any = inject('viewStacks');
@@ -143,7 +142,6 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({
const customAnimation = enteringViewItem.routerAnimation;
if (
animationBuilder === undefined &&
// todo check for tab switch
customAnimation !== undefined
) {
animationBuilder = customAnimation;
@@ -198,7 +196,7 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({
const transition = (
enteringEl: HTMLElement,
leavingEl: HTMLElement,
direction: any, // TODO types
direction: any,
showGoBack: boolean,
progressAnimation: boolean,
animationBuilder?: AnimationBuilder
@@ -307,7 +305,6 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information.
if (
animationBuilder === undefined &&
routerDirection === 'back' &&
// todo check for tab switch
customAnimation !== undefined
) {
animationBuilder = customAnimation;
@@ -425,7 +422,6 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information.
*/
onUnmounted(() => viewStacks.clear(id));
// TODO types
const registerIonPage = (viewItem: any, ionPageEl: HTMLElement) => {
const oldIonPageEl = viewItem.ionPageElement;
@@ -471,7 +467,6 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information.
return h(
'ion-router-outlet',
{ ref: 'ionRouterOutlet' },
// TODO types
components && components.map((c: any) => {
let props = {
ref: c.vueComponentRef,

View File

@@ -25,7 +25,7 @@ export const IonicVue: Plugin = {
* By default Ionic Framework hides elements that
* are not hydrated, but in the CE build there is no
* hydration.
* TODO: Remove when all integrations have been
* TODO FW-2797: Remove when all integrations have been
* migrated to CE build.
*/
if (typeof (document as any) !== 'undefined') {

View File

@@ -24,7 +24,6 @@ export const generateId = (type = 'main') => {
return (id).toString();
};
// TODO types
export const fireLifecycle = (vueComponent: any, vueInstance: Ref<ComponentPublicInstance>, lifecycle: LIFECYCLE_EVENTS) => {
if (vueComponent?.[lifecycle]) {
vueComponent[lifecycle].bind(vueInstance?.value)();