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

Remove data-value #895

Open
thet opened this issue Sep 9, 2021 · 2 comments
Open

Remove data-value #895

thet opened this issue Sep 9, 2021 · 2 comments
Assignees

Comments

@thet
Copy link
Member

thet commented Sep 9, 2021

Problem

While working on pat-tiptap I noticed that the current value of the textarea is always replicated in a data-value attribute. This can be quite long for big text documents and unnecessary JavaScript execution on each textarea value's update. Also, the DOM structure becomes unreadable due to this big data chunk in a DOM node's attribute.

pat-focus sets both the has-value class and the data-value attribute.

Options to solve the problem

  1. Remove the functionality which updates data-value and/or has-value.
  2. Add a method to explicitly include or explicitly exclude the update of data-value.

If possible, I'd opt for option 1) to reduce complexity.

Usage of data-value

When I search for data value through ploneintranet.prototype's code base I find these occurrences:

assets/quaive/script/bundle.js
30190:      $relatives.addClass("has-value").attr("data-value", el.value);
30200:      }).removeClass("has-value").attr("data-value", null);

_layouts/apps/calendar/calendar.html
135:                                    <input id="calendar-central-calendars-asian-steering-committee" type="checkbox" data-value="asian-steering-committee" checked />

_sass/components/_voting-bar.scss
99:		&[data-value=option-1] {
112:		&[data-value=option-2] {
125:		&[data-value=option-3] {

_sass/components/_menu-list.scss
55:        // &[data-value='draft'] {
59:        // &[data-value='public'] {
63:        // &[data-value='published'] {

_sass/components/_sticky-note.scss
408:		[data-value="sticky-note-kind-offered"] ~ .button-bar:before {
412:		[data-value="sticky-note-kind-needed"] ~ .button-bar:before {

_sass/components/_canvas-toolbar.scss
694:        &[data-value='draft'],
695:        &[data-value='retract'] {
704:        &[data-value='public'] {
708:        &[data-value='pending'],
709:        &[data-value='for-review'] {
720:        &[data-value='retract'] {
731:        &[data-value='Public'] {
736:        &[data-value='published'] {
744:        &[data-value='running'] {
752:        &[data-value='closed'] {

In OiRA, it's not used at all:

assets/oira/script/bundle.js
25898:      $relatives.addClass("has-value").attr("data-value", el.value);
25908:      }).removeClass("has-value").attr("data-value", null);

_sass/components/_menu-list.scss
55:        // &[data-value='draft'] {
59:        // &[data-value='public'] {
63:        // &[data-value='published'] {

Usage of has-value

in ploneintranet.prototype:

assets/quaive/script/bundle.js
30190:      $relatives.addClass("has-value").attr("data-value", el.value);
30200:      }).removeClass("has-value").attr("data-value", null);

_sass/components/_canvas-toolbar.scss
1369:		&.has-value {

_sass/components/_document-body.scss
253:      &.has-value {

_sass/libraries/patterns/components/_toolbar.scss
374:		&.has-value,

_layouts/apps/calendar/calendar.html
131:                                <label class="cal-cat-asian-steering-committee workspace has-value {% if page.workspace_name == 'Open Market Committee' %}current{% endif %}"

in oira.prototype:

assets/oira/script/bundle.js
25898:      $relatives.addClass("has-value").attr("data-value", el.value);
25908:      }).removeClass("has-value").attr("data-value", null);

_sass/components/_button-bar.scss
162:.pat-checklist.pat-clone.has-value + .button-bar.cloning {

_sass/components/_checklist.scss
242:	.measures-in-place.has-value ~ & {
300:			&.has-value {
309:		&.has-value {

_sass/libraries/patterns/components/_toolbar.scss
489:		&.has-value,
@cornae
Copy link
Member

cornae commented Sep 9, 2021

Good point. However for these large documents we will typically use real time collaboration in the phase two of the tiptap story.

If we have to do something now then I would opt for option 2), because I use data-value and might use it more in the future. Also, I intend to simplify instances of pat content mirror that we will keep using by a data-value based version of content mirror.

However, I don't see an emergency to look at this right now. All performs adequately.

@thet thet changed the title Remove data-value and Remove data-value Sep 14, 2021
@pilz
Copy link
Member

pilz commented Oct 1, 2021

@thet let's first talk about the impact and need and cost balance

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

No branches or pull requests

3 participants