EZP-31632: Floating table toolbar #148

SerheyDolgushev · 2020-05-15T11:31:58Z

Question	Answer
JIRA issue	EZP-31632
Bug/Improvement	yes
New feature	no
Target version	master
BC breaks	no
Tests pass	yes
Doc needed	no

Right now table toolbar has fixed positioning, and in some cases it brings inconvenience.

Steps to reproduce:

Open any content with RichtText field for editing
Insert a big table (50 rows) into RichText field
Scroll to the last table row and try to insert a new row after it

Expected result
The table toolbar should be shown on the screen, and new row should be inserted without any scrolling.

Actual result
The table toolbar remains at the start of the table. And the editor needs to scroll to it, to add a new row.

TODO:

Implement feature / fix a bug.
Implement tests.
Fix new code according to Coding Standards ($ composer fix-cs).
Ask for Code Review.

dew326 · 2020-05-15T11:46:59Z

I think this bug is also in the 2.5 LTS. Please create this fix in ezplatform-admin-ui to branch 1.5 or if you feel it more like improvement please change the base branch to master here because it's questionable if it is bug or improvement.

SerheyDolgushev · 2020-05-18T12:16:45Z

@dew326 Changed base branch to master, as personally I think it is not a bugfix, but an improvement.

dew326 · 2020-05-18T13:21:29Z

src/bundle/Resources/public/js/OnlineEditor/toolbars/config/base-table.js


-export default class EzConfigTableBase extends EzConfigButtonsBase {
+export default class EzConfigTableBase extends EzConfigBase {


I'm not sure but EzConfigFixedBase could be better because as I understand this fix will work for td and it's at the top of the td. When user will use elements path and will select table there will be the same problem. With EzConfigFixedBase it will be fixed at the top o the screen if it should be outside.

Also, the second question is do we want to cover the table with toolbar (I know there was some clients that don't like when toolbar is covering what they wrote in previous line), but this is a question to @SylvainGuittard.

I just tried the steps to reproduce and I understand the pain point here.
I think it makes sense for the table toolbar to be visible by the user.

Hi guys, adding the UX perspective here.
I would go for a similar solution that the RTE Paragraph toolbar has right now, given this behavior was added after being user tested, I agree @dew326.
Maybe a potential option to make this work better would be also to remove the fixed position from ez-edit-header__content-type-name.

neiluJ · 2020-09-17T14:12:34Z

Hello guys, any release date planned for this fix ?

SerheyDolgushev · 2020-09-22T21:19:57Z

Hello guys, any release date planned for this fix ?

@neiluJ meantime you can use https://packagist.org/packages/contextualcode/ezplatform-alloyeditor-table-tools

dew326 · 2020-09-23T07:09:43Z

For this to be merged we need:

Rebase with current master
Change the EzConfigBase to EzConfigFixedBase as this behavior is better. (EZP-31632: Floating table toolbar #148 (comment))

SerheyDolgushev · 2020-09-23T13:32:40Z

For this to be merged we need:

Rebase with current master

Change the EzConfigBase to EzConfigFixedBase as this behavior is better. (#148 (comment))

Latest master is merged
Done: 402bc7a

dew326 · 2020-09-24T06:04:21Z

Could you also explain this commit? cff502a
Seems like it fixing some bug, which is not only in master?

SerheyDolgushev · 2020-09-29T19:25:43Z

@dew326
Sorry, I`m not sure why cff502a is a part of this PR, as it is related to #149. Most likely I realized it after #149 was merged, but still I do not remember why this commit was pushed to this PR. Also, I just pushed b676083. Which is directly related to #163.

Should both of those commits be removed from this PR? And should we create a separate PR for each of them?

Also please note 43f5ad8. And the reason for this, is that when EzConfigFixedBase is used the toolbar is shown at the top of the table. But when EzConfigBase is used the toolbar is shown at the active table row. And that is the main purpose of this PR.

SerheyDolgushev · 2020-09-29T19:45:35Z

Sorry, but right now I have a feeling like cff502a and b676083 are making all the things much more complicated and they should be removed from this PR. A separate PR will be submitted for them. I'm not going to remove them from this PR, but will just revert, to make sure their links will remain in the PR conversation messages.

SerheyDolgushev · 2020-09-29T20:08:51Z

#165 was submitted, and it is absolutely unrelated to this PR.

dew326 · 2020-09-30T07:00:30Z

Also please note 43f5ad8. And the reason for this, is that when EzConfigFixedBase is used the toolbar is shown at the top of the table. But when EzConfigBase is used the toolbar is shown at the active table row. And that is the main purpose of this PR.

We had complaints about that the toolbar covers the previous text and this was the reason why we introduced this fixed toolbar. Yes, it will go to the top of a table but when the top of the table is outside of the screen it will be fixed to the top of the window.

SerheyDolgushev · 2020-09-30T12:53:04Z

@dew326 Sorry, but I just did another test using EzConfigFixedBase using the case from this PR description. And the toolbar wasn't shown at all when "the top of the table is outside of the screen":

In that case, maybe something needs to be fixed in EzConfigFixedBase?

dew326 · 2020-10-01T05:39:43Z

It works for me:

You have some fixed bar at the top, which I don't recall if we have something like this in our product. Maybe this bar is covering the alloyEditor toolbar?

SerheyDolgushev · 2020-10-06T12:39:57Z

@dew326 EzConfigFixedBase was not working for me because of missing ae-toolbar-floating CSS class.

So EzConfigFixedBase was enabled: c1fd697. And missing ae-toolbar-floating CSS was fixed: 2e6acfe.

Also I was checking this PR on 3.1.3 and found out that the toolbar is covered by "edit header", which seems to be not a case for latest master because of the new UI changes. It is fixed in 16d73b0.

src/bundle/Resources/public/js/OnlineEditor/toolbars/config/base-fixed.js

SylvainGuittard · 2020-10-08T12:36:03Z

Thanks @SerheyDolgushev for your contribution.

I think this bug is also in the 2.5 LTS. Please create this fix in ezplatform-admin-ui to branch 1.5 or if you feel it more like improvement please change the base branch to master here because it's questionable if it is bug or improvement.

Sorry, I am late in the game. I agree with Darek. I consider this as a bug. IMHO, this fix should also go to 2.5LTS
Can we merge and backport or should we rebase again 1.5?

dew326 · 2020-10-08T12:38:18Z

In 2.5LTS we had AlloyEditor in the admin-ui bundle so another PR should be opened in this repo against branch 1.5.
This PR should be rebased with the latest supported branch so with 2.1.

This reverts commit cff502a.

This reverts commit 16d73b0.

This reverts commit 9147515.

SerheyDolgushev · 2020-10-09T12:40:21Z

@dew326 this PR is based on 2.1 now, and the "Edit header" offset was enabled. Please let me know if any changes are required here.

dew326 · 2020-10-09T13:22:05Z

No, this PR is good.

dew326 · 2020-10-12T06:09:36Z

I will pass this PR to the QA.

@SerheyDolgushev Could you prepare the same fix for 2.5 (in admin-ui bundle)?

dew326 · 2020-10-12T06:14:26Z

Note for QA:
This PR should also fix customer request in 3.1 https://jira.ez.no/browse/EZP-32006

katarzynazawada

Toolbar is shown when scrolling but it skips for a while when selecting another cell in the table. See please: http://g.recordit.co/CxmDj9xf75.gif

SerheyDolgushev · 2020-10-23T11:06:19Z

Sorry, but it seems like the lag is caused by the huge size of the test table. And it seems like that lag is not related to this PR. @dew326 have you had any chance to look into this? Maybe there should be a separate PR/issue for toolbar rendering performance on huge tables like the one from the last QA?

micszo

Hi @SerheyDolgushev
I tested these changes on v3.1.4.

Even with a small table (5x5) the issue exists.

Actual: Toolbar is first aligned to the table cell then, after a while, toolbar is aligned to the table (to the left)
Expected: Toolbar is aligned to the table (to the left, as for a long portion of text) from the beginning.

Regarding the bug https://issues.ibexa.co/browse/EZP-32006 we might need to deal with it separately. \cc @barw4

SerheyDolgushev · 2020-12-02T14:28:19Z

Hello @micszo

Could you please test it now?

micszo · 2020-12-03T14:57:06Z

Hi @SerheyDolgushev

the sticky toolbar works fine now for long portions of text and large tables.

However the pop-up with settings for custom tags (youtube, facebook, twitter) is broken now.
Please see the recording: https://recordit.co/7cLmNWL37l (the complete animation didn't get recorded)

Steps:

Add e.g. youtube custom tag.
Fill URL.
Save
Try to open custom tag settings with edit pencil.

Actual result: The pop-up jumps out of the visible area.
Expected result: The pop-up is displayed above the custom tag.

SerheyDolgushev · 2020-12-03T16:14:41Z

Hello @micszo,
Can you please check all the possible usecases one more time? Also please note, that there are separate issues with the custom tag toolbar, which was fixed in #169.

micszo

The changes request in https://issues.ibexa.co/browse/EZP-31632 and https://issues.ibexa.co/browse/EZP-32006 are in place now.

Didn't find new issues with the toolbars this round.

Checked both block and inline custom tags.

Verified on eZ Platform EE v3.1.4 with diff. Also applied the diff for https://issues.ibexa.co/browse/EZP-32187.

lserwatka · 2020-12-07T09:16:46Z

@dew326 could you merge it up?

dew326 requested review from dew326, GrabowskiM and lucasOsti May 15, 2020 11:34

SerheyDolgushev changed the base branch from 2.0 to master May 18, 2020 12:15

dew326 reviewed May 18, 2020

View reviewed changes

SerheyDolgushev mentioned this pull request Sep 29, 2020

Fixes #149 and #163 #165

Merged

4 tasks

SerheyDolgushev requested a review from dew326 October 6, 2020 12:40

dew326 requested changes Oct 7, 2020

View reviewed changes

src/bundle/Resources/public/js/OnlineEditor/toolbars/config/base-fixed.js Show resolved Hide resolved

dew326 approved these changes Oct 7, 2020

View reviewed changes

lucasOsti approved these changes Oct 7, 2020

View reviewed changes

GrabowskiM approved these changes Oct 7, 2020

View reviewed changes

katarzynazawada self-assigned this Oct 7, 2020

katarzynazawada added the Ready for QA label Oct 7, 2020

katarzynazawada removed the Ready for QA label Oct 8, 2020

SerheyDolgushev added 5 commits October 9, 2020 13:34

Revert "Fix related to ezsystems#149"

84ca0df

This reverts commit cff502a.

EZP-31632: Use EzConfigFixedBase for base table toolbar

77bf149

EZP-31632: Add ae-toolbar-floating class to base table toolbar

dbc9426

EZP-31632: Added "edit header" offset

ea7b173

Revert "EZP-31632: Added "edit header" offset"

05df450

This reverts commit 16d73b0.

SerheyDolgushev changed the base branch from master to 2.1 October 9, 2020 12:35

SerheyDolgushev force-pushed the fixed-table-toolbar-positioning branch from 9147515 to 05df450 Compare October 9, 2020 12:35

SerheyDolgushev added 2 commits October 9, 2020 13:37

Revert ez-btn-linkedit.js

c91bcb5

Revert "Revert "EZP-31632: Added "edit header" offset""

4b5dde5

This reverts commit 9147515.

katarzynazawada requested changes Oct 12, 2020

View reviewed changes

micszo assigned micszo and unassigned katarzynazawada Dec 1, 2020

micszo requested changes Dec 2, 2020

View reviewed changes

EZP-31632: Fix table cell toolbar

aab413f

EZP-31632: fix toolbar for custom tags

f6344b4

micszo approved these changes Dec 4, 2020

View reviewed changes

micszo requested a review from katarzynazawada December 4, 2020 15:07

micszo added the QA approved label Dec 4, 2020

micszo removed their assignment Dec 4, 2020

lserwatka merged commit 5b7cc89 into ezsystems:2.1 Dec 7, 2020

mateuszdebinski mentioned this pull request Mar 11, 2021

EZP-31632: Floating table toolbar ezsystems/ezplatform-admin-ui#1721

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-31632: Floating table toolbar #148

EZP-31632: Floating table toolbar #148

SerheyDolgushev commented May 15, 2020 •

edited

Loading

dew326 commented May 15, 2020 •

edited

Loading

SerheyDolgushev commented May 18, 2020

dew326 May 18, 2020

SylvainGuittard May 18, 2020

inakijv May 18, 2020 •

edited

Loading

neiluJ commented Sep 17, 2020

SerheyDolgushev commented Sep 22, 2020

dew326 commented Sep 23, 2020

SerheyDolgushev commented Sep 23, 2020

dew326 commented Sep 24, 2020

SerheyDolgushev commented Sep 29, 2020

SerheyDolgushev commented Sep 29, 2020

SerheyDolgushev commented Sep 29, 2020

dew326 commented Sep 30, 2020

SerheyDolgushev commented Sep 30, 2020

dew326 commented Oct 1, 2020

SerheyDolgushev commented Oct 6, 2020

SylvainGuittard commented Oct 8, 2020

dew326 commented Oct 8, 2020 •

edited

Loading

SerheyDolgushev commented Oct 9, 2020

dew326 commented Oct 9, 2020

dew326 commented Oct 12, 2020

dew326 commented Oct 12, 2020

katarzynazawada left a comment

SerheyDolgushev commented Oct 23, 2020

micszo left a comment •

edited

Loading

SerheyDolgushev commented Dec 2, 2020

micszo commented Dec 3, 2020

SerheyDolgushev commented Dec 3, 2020 •

edited

Loading

micszo left a comment

lserwatka commented Dec 7, 2020


		export default class EzConfigTableBase extends EzConfigButtonsBase {
		export default class EzConfigTableBase extends EzConfigBase {

EZP-31632: Floating table toolbar #148

EZP-31632: Floating table toolbar #148

Conversation

SerheyDolgushev commented May 15, 2020 • edited Loading

dew326 commented May 15, 2020 • edited Loading

SerheyDolgushev commented May 18, 2020

dew326 May 18, 2020

Choose a reason for hiding this comment

SylvainGuittard May 18, 2020

Choose a reason for hiding this comment

inakijv May 18, 2020 • edited Loading

Choose a reason for hiding this comment

neiluJ commented Sep 17, 2020

SerheyDolgushev commented Sep 22, 2020

dew326 commented Sep 23, 2020

SerheyDolgushev commented Sep 23, 2020

dew326 commented Sep 24, 2020

SerheyDolgushev commented Sep 29, 2020

SerheyDolgushev commented Sep 29, 2020

SerheyDolgushev commented Sep 29, 2020

dew326 commented Sep 30, 2020

SerheyDolgushev commented Sep 30, 2020

dew326 commented Oct 1, 2020

SerheyDolgushev commented Oct 6, 2020

SylvainGuittard commented Oct 8, 2020

dew326 commented Oct 8, 2020 • edited Loading

SerheyDolgushev commented Oct 9, 2020

dew326 commented Oct 9, 2020

dew326 commented Oct 12, 2020

dew326 commented Oct 12, 2020

katarzynazawada left a comment

Choose a reason for hiding this comment

SerheyDolgushev commented Oct 23, 2020

micszo left a comment • edited Loading

Choose a reason for hiding this comment

SerheyDolgushev commented Dec 2, 2020

micszo commented Dec 3, 2020

SerheyDolgushev commented Dec 3, 2020 • edited Loading

micszo left a comment

Choose a reason for hiding this comment

lserwatka commented Dec 7, 2020

SerheyDolgushev commented May 15, 2020 •

edited

Loading

dew326 commented May 15, 2020 •

edited

Loading

inakijv May 18, 2020 •

edited

Loading

dew326 commented Oct 8, 2020 •

edited

Loading

micszo left a comment •

edited

Loading

SerheyDolgushev commented Dec 3, 2020 •

edited

Loading