-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat(): Missing features from fabric 5 and types exports #8954
Changes from all commits
936f118
d0dd06d
133b4c9
3b03394
9c4d39a
96be120
a782f6b
a163b6b
1231b1e
c330ac9
f342ba1
17d2165
42aa71c
99da8d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1319,17 +1319,16 @@ export class StaticCanvas< | |
if (!isTextObject(obj)) { | ||
return; | ||
} | ||
let fontFamily = obj.fontFamily; | ||
const { styles, fontFamily } = obj; | ||
if (fontList[fontFamily] || !fontPaths[fontFamily]) { | ||
return; | ||
} | ||
fontList[fontFamily] = true; | ||
if (!obj.styles) { | ||
if (!styles) { | ||
return; | ||
} | ||
Object.values(obj.styles).forEach((styleRow) => { | ||
Object.values(styleRow).forEach((textCharStyle) => { | ||
fontFamily = textCharStyle.fontFamily; | ||
Object.values(styles).forEach((styleRow) => { | ||
Object.values(styleRow).forEach(({ fontFamily = '' }) => { | ||
Comment on lines
+1322
to
+1331
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have touched this in my cleanup #8952 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think the only thing i really changed is es6 leverage |
||
if (!fontList[fontFamily] && fontPaths[fontFamily]) { | ||
fontList[fontFamily] = true; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,25 +6,10 @@ import type { | |
} from '../Object/types'; | ||
import { FabricObject } from '../Object/FabricObject'; | ||
import { styleProperties } from './constants'; | ||
import type { StylePropertiesType } from './constants'; | ||
import type { Text } from './Text'; | ||
|
||
export type TextStyleDeclaration = Partial< | ||
Pick< | ||
Text, | ||
| 'fill' | ||
| 'stroke' | ||
| 'strokeWidth' | ||
| 'fontSize' | ||
| 'fontFamily' | ||
| 'fontWeight' | ||
| 'fontStyle' | ||
| 'textBackgroundColor' | ||
| 'deltaY' | ||
| 'overline' | ||
| 'underline' | ||
| 'linethrough' | ||
> | ||
>; | ||
export type TextStyleDeclaration = Partial<Pick<Text, StylePropertiesType>>; | ||
|
||
export type TextStyle = { | ||
[line: number | string]: { [char: number | string]: TextStyleDeclaration }; | ||
|
@@ -38,7 +23,7 @@ export abstract class StyledText< | |
declare abstract styles: TextStyle; | ||
protected declare abstract _textLines: string[][]; | ||
protected declare _forceClearCache: boolean; | ||
static _styleProperties = styleProperties; | ||
static _styleProperties: Readonly<StylePropertiesType[]> = styleProperties; | ||
abstract get2DCursorLocation( | ||
selectionStart: number, | ||
skipWrapping?: boolean | ||
|
@@ -78,8 +63,8 @@ export abstract class StyledText< | |
* @param {Number} lineIndex to check the style on | ||
* @return {Boolean} | ||
*/ | ||
styleHas(property: string, lineIndex?: number): boolean { | ||
if (!this.styles || !property || property === '') { | ||
styleHas(property: keyof TextStyleDeclaration, lineIndex?: number): boolean { | ||
if (!this.styles) { | ||
return false; | ||
} | ||
if (typeof lineIndex !== 'undefined' && !this.styles[lineIndex]) { | ||
|
@@ -111,8 +96,8 @@ export abstract class StyledText< | |
* | ||
* @param {string} property The property to compare between characters and text. | ||
*/ | ||
cleanStyle(property: string) { | ||
if (!this.styles || !property || property === '') { | ||
cleanStyle(property: keyof TextStyleDeclaration) { | ||
if (!this.styles) { | ||
return false; | ||
} | ||
const obj = this.styles; | ||
|
@@ -124,12 +109,8 @@ export abstract class StyledText< | |
for (const p1 in obj) { | ||
letterCount = 0; | ||
for (const p2 in obj[p1]) { | ||
const styleObject = obj[p1][p2], | ||
// TODO: this shouldn't be necessary anymore with modern browsers | ||
stylePropertyHasBeenSet = Object.prototype.hasOwnProperty.call( | ||
styleObject, | ||
property | ||
); | ||
const styleObject = obj[p1][p2] || {}, | ||
stylePropertyHasBeenSet = styleObject[property] !== undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #8953 => but the rest are typeof meanwhile. |
||
|
||
stylesCount++; | ||
|
||
|
@@ -164,6 +145,7 @@ export abstract class StyledText< | |
graphemeCount += this._textLines[i].length; | ||
} | ||
if (allStyleObjectPropertiesMatch && stylesCount === graphemeCount) { | ||
// @ts-expect-error conspiracy theory of TS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And don't know if to laugh or cry |
||
this[property as keyof this] = stylePropertyValue; | ||
this.removeStyle(property); | ||
} | ||
|
@@ -176,8 +158,8 @@ export abstract class StyledText< | |
* | ||
* @param {String} props The property to remove from character styles. | ||
*/ | ||
removeStyle(property: string) { | ||
if (!this.styles || !property || property === '') { | ||
removeStyle(property: keyof TextStyleDeclaration) { | ||
if (!this.styles) { | ||
return; | ||
} | ||
const obj = this.styles; | ||
|
@@ -289,6 +271,7 @@ export abstract class StyledText< | |
styleProps = (this.constructor as typeof StyledText)._styleProperties; | ||
for (let i = 0; i < styleProps.length; i++) { | ||
const prop = styleProps[i]; | ||
// @ts-expect-error TS complains even when we serve everything on a silver plate. | ||
styleObject[prop] = | ||
typeof style[prop] === 'undefined' | ||
? this[prop as keyof this] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShaMan123 let's have the build error on default if types are broken.
We can comment this locally when we need to work in a broken state with watchers, but i feel better if the pipeline is strict about this, otherwise we found ourselves with unexpected cleanups that we can't see when we code review the PRs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree
That sourcemap warning must be resoled as well