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

task(IText): rename fabric-data attr #8292

Merged
merged 1 commit into from
Sep 20, 2022
Merged
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 src/mixins/itext_key_behavior.mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { config } from '../config';
this.hiddenTextarea.setAttribute('autocorrect', 'off');
this.hiddenTextarea.setAttribute('autocomplete', 'off');
this.hiddenTextarea.setAttribute('spellcheck', 'false');
this.hiddenTextarea.setAttribute('data-fabric-hiddentextarea', '');
this.hiddenTextarea.setAttribute('data-fabric', 'textarea');
Copy link
Member

@asturur asturur Sep 19, 2022

Choose a reason for hiding this comment

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

i have no much to say about this change, is neutral, but i want to point out a few things.
data-* does cover both data-fabric and data-fabric-attribute.
The assumed consistency gain is killed by the required change of code of whoever is using this.
This is really just a string, an identified, we can change it yes.
But i do think is a waste of energy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

point taken
I came across it by chance
I view it as something less to document
Need we can say "use the querySelector data-fabric to get fabric elements", the value is one of...
I didn't think of the option data-fabric-*...

this.hiddenTextarea.setAttribute('wrap', 'off');
var style = this._calcTextareaPosition();
// line-height: 1px; was removed from the style to fix this:
Expand Down