Skip to content

Conversation

@Arantiryo
Copy link
Owner

No description provided.

* @private
*/
export function _computeSegments(line, segmentOptions) {
export function _computeSegments(line: LineElement, segmentOptions: any) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Как типизировать SegmentOptions? @dangreen

Choose a reason for hiding this comment

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

наверное надо сначала перевести на ts helpers.config.js, так как там эта тема с контекстами

Copy link

Choose a reason for hiding this comment

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

ну тогда пока просто SegmentStyle получается

start: number;
end: number;
loop: boolean;
style?: AnyObject;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Segment.style - я добавил как AnyObject, но может есть вариант более конкретный? Вроде в readStyle как раз такой объект описывается, но я не уверен. @dangreen

Choose a reason for hiding this comment

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

function readStyle(options) {
  return {
    backgroundColor: options.backgroundColor,
    borderCapStyle: options.borderCapStyle,
    borderDash: options.borderDash,
    borderDashOffset: options.borderDashOffset,
    borderJoinStyle: options.borderJoinStyle,
    borderWidth: options.borderWidth,
    borderColor: options.borderColor
  };
}

там вот эти пропсы скорей всего

* @private
*/
export function _boundSegments(line, bounds) {
export function _boundSegments(line: LineElement, bounds: Bounds) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Типы Point и PointElement, в LineElement используется первый, но из кода вроде получается, что должен быть второй. @dangreen

Choose a reason for hiding this comment

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

да, там PointElement, надо поправить

@Arantiryo
Copy link
Owner Author

И хотел спросить, как типизировать приватные поля, которые начинаются с "_", например line._loop.

_loop: boolean;
_fullLoop: boolean;
_datasetIndex: number;
_chart: AnyObject;
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen какой здесь тип использовать для _chart, вот этот?

Choose a reason for hiding this comment

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

@Arantiryo Да

* @private
*/
export function _measureText(ctx, data, gc, longest, string) {
export function _measureText(ctx: CanvasRenderingContext2D, data, gc, longest: number, string) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen Дэн, не нашел в коде, что конкретно туда передается, можешь подсказать?

*/
export function _longestText(ctx, font, arrayOfThings, cache) {
// eslint-disable-next-line complexity
export function _longestText<T>(ctx: CanvasRenderingContext2D, font, arrayOfThings: T[], cache) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen Как лучше типизировать arrayOfThings? И с кэшем не совсем понятно, что делать.

* @private
*/
export function _steppedLineTo(ctx, previous, target, flip, mode) {
export function _steppedLineTo(ctx: CanvasRenderingContext2D, previous: Point, target: Point, flip, mode) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen flip не нашел типы для mode, подскажи, плиз

}

function drawBackdrop(ctx, opts) {
function drawBackdrop(ctx: CanvasRenderingContext2D, opts: BackdropOptions) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen вроде бы параметры типизированы корректно, но в строке ctx.fillStyle = opts.color; ошибка несоответствия типов.

* @param {*} rect Bounding rect
*/
export function addRoundedRectPath(ctx, rect) {
export function addRoundedRectPath(ctx: CanvasRenderingContext2D, rect: RoundedRect) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen Здесь вроде бы тоже с типами понятно, но ругается на тип radius, хотя должно быть все ок на первый взгляд

Copy link
Owner Author

Choose a reason for hiding this comment

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

Property 'topLeft' does not exist on type 'CornerRadius'.
Property 'topLeft' does not exist on type 'number'.ts(2339)

@dangreen
Copy link

@Arantiryo а зачем ты тут канвас переводишь на TS? тут же мы только segment делаем

плюс я уже сделал канвас chartjs#11100

@Arantiryo
Copy link
Owner Author

Справедливо, надо было сначала. с тобой посоветоваться, что дальше делать. Я начал helpers.config.js переписывать, но там все совсем запутано оказалось, поэтому решил канвас начать.

Тогда я откачу последний коммит. Здесь остался #1 (comment) неразрешенный только, но он завязан на helpers.config.js. Создать тогда ПР в основной репозиторий без последнего коммита? @dangreen

Copy link

@dangreen dangreen left a comment

Choose a reason for hiding this comment

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

сделай плиз откат канваса

Comment on lines +1820 to 1824
_loop: boolean;
_fullLoop: boolean;
_datasetIndex: number;
_chart: Chart;
}
Copy link

Choose a reason for hiding this comment

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

они наверное private?

Copy link
Owner Author

Choose a reason for hiding this comment

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

То есть их нужно убрать из типа? Просто тогда ТС при их использовании начинает ругаться, что такого свойства нет.

Copy link

Choose a reason for hiding this comment

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

я имел в виду модифигатор private

но ладно, оставляй как есть

start: number;
end: number;
loop: boolean;
style?: LineOptions['segment'];
Copy link

Choose a reason for hiding this comment

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

вынеси это в интерфейс SegmentStyle, и его везде используй

Copy link
Owner Author

Choose a reason for hiding this comment

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

Готово 👍

* @private
*/
export function _computeSegments(line, segmentOptions) {
export function _computeSegments(line: LineElement, segmentOptions: any) {
Copy link

Choose a reason for hiding this comment

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

ну тогда пока просто SegmentStyle получается

Comment on lines 13 to 17
/**
* @typedef { import('../elements/element.line.js').default } LineElement
* @typedef { import('../elements/element.point.js').default } PointElement
* @typedef {{start: number, end: number, loop: boolean, style?: any}} Segment
*/
Copy link

Choose a reason for hiding this comment

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

надо тоже на ts переделать или убрать

Copy link
Owner Author

Choose a reason for hiding this comment

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

Убрал

Comment on lines 7 to 11
interface Bounds {
property: string;
start: number;
end: number;
}
Copy link

Choose a reason for hiding this comment

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

перенеси в файл с типами, мб как то SegmentBounds назвать лучше

Copy link
Owner Author

Choose a reason for hiding this comment

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

Готово

Comment on lines 1693 to 1699
backgroundColor: Scriptable<Color|undefined, ScriptableLineSegmentContext>,
borderColor: Scriptable<Color|undefined, ScriptableLineSegmentContext>,
borderCapStyle: Scriptable<CanvasLineCap|undefined, ScriptableLineSegmentContext>;
borderDash: Scriptable<number[]|undefined, ScriptableLineSegmentContext>;
borderDashOffset: Scriptable<number|undefined, ScriptableLineSegmentContext>;
borderJoinStyle: Scriptable<CanvasLineJoin|undefined, ScriptableLineSegmentContext>;
borderWidth: Scriptable<number|undefined, ScriptableLineSegmentContext>;
Copy link

Choose a reason for hiding this comment

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

Suggested change
backgroundColor: Scriptable<Color|undefined, ScriptableLineSegmentContext>,
borderColor: Scriptable<Color|undefined, ScriptableLineSegmentContext>,
borderCapStyle: Scriptable<CanvasLineCap|undefined, ScriptableLineSegmentContext>;
borderDash: Scriptable<number[]|undefined, ScriptableLineSegmentContext>;
borderDashOffset: Scriptable<number|undefined, ScriptableLineSegmentContext>;
borderJoinStyle: Scriptable<CanvasLineJoin|undefined, ScriptableLineSegmentContext>;
borderWidth: Scriptable<number|undefined, ScriptableLineSegmentContext>;
backgroundColor: Scriptable<Color | undefined, ScriptableLineSegmentContext>,
borderColor: Scriptable<Color | undefined, ScriptableLineSegmentContext>,
borderCapStyle: Scriptable<CanvasLineCap | undefined, ScriptableLineSegmentContext>;
borderDash: Scriptable<number[] | undefined, ScriptableLineSegmentContext>;
borderDashOffset: Scriptable<number | undefined, ScriptableLineSegmentContext>;
borderJoinStyle: Scriptable<CanvasLineJoin | undefined, ScriptableLineSegmentContext>;
borderWidth: Scriptable<number | undefined, ScriptableLineSegmentContext>;

Copy link
Owner Author

Choose a reason for hiding this comment

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

Поправил

@Arantiryo
Copy link
Owner Author

@dangreen Дэн, еще есть ошибка 'readStyle' was used before it was defined. eslint[@typescript-eslint/no-use-before-define] для нескольких функций, нужно поправить ее?

@dangreen
Copy link

dangreen commented Feb 6, 2023

@Arantiryo для всего файла правило отключи

* @param {object} [bounds]
* @param {string} bounds.property - the property of a `PointElement` we are bounding. `x`, `y` or `angle`.
* @param {number} bounds.start - start value of the property
* @param {number} bounds.end - end value of the property
Copy link

@dangreen dangreen Feb 6, 2023

Choose a reason for hiding this comment

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

из jsdoc надо типы убрать кстати

Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen убрал типы, оставил только описания

Comment on lines 64 to 74
* @param {object} segment
* @param {number} segment.start - start index of the segment, referring the points array
* @param {number} segment.end - end index of the segment, referring the points array
* @param {boolean} segment.loop - indicates that the segment is a loop
* @param {object} [segment.style] - segment style
* @param {PointElement[]} points - the points that this segment refers to
* @param {object} [bounds]
* @param {string} bounds.property - the property of a `PointElement` we are bounding. `x`, `y` or `angle`.
* @param {number} bounds.start - start value of the property
* @param {number} bounds.end - end value of the property
* @private
Copy link

Choose a reason for hiding this comment

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

нужно было удалить только типы, описания аргументов нужно оставить

* @return {Segment[]}
* @param line
* @param segmentOptions
* @return Segment[]
Copy link

Choose a reason for hiding this comment

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

тут и ниже видимо ошибся, просто удали строчки "@return Segment[]" раз там описания нет

Copy link

Choose a reason for hiding this comment

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

и можешь делать пр

Copy link
Owner Author

Choose a reason for hiding this comment

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

@dangreen Сделал, спасибо!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants