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

There's no way to access table properties when table longer than the viewport #6190

Closed
oleq opened this issue Feb 4, 2020 · 1 comment · Fixed by #6810
Closed

There's no way to access table properties when table longer than the viewport #6190

oleq opened this issue Feb 4, 2020 · 1 comment · Fixed by #6810
Assignees
Labels
domain:accessibility This issue reports an accessibility problem. domain:ui/ux This issue reports a problem related to UI or UX. package:table type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Feb 4, 2020

📝 Provide detailed reproduction steps (if any)

A follow-up of #6049 and #6112.

  1. Create a table longer than the viewport.
  2. Try to open the table (not cell) properties form.

✔️ Expected result

It should be visible somewhere and accessible.

❌ Actual result

It is pinned either to the top or to the bottom, way beyond the visible viewport and user's reach.

📃 Other details

image


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. package:table domain:ui/ux This issue reports a problem related to UI or UX. labels Feb 4, 2020
@oleq oleq added this to the next milestone Feb 4, 2020
@jodator jodator modified the milestones: next, iteration 32 Apr 30, 2020
@jodator jodator assigned jodator and oleq and unassigned jodator Apr 30, 2020
@oleq
Copy link
Member Author

oleq commented May 12, 2020

I managed to implement the following UX

using this short code:

diff --git a/packages/ckeditor5-table/src/ui/utils.js b/packages/ckeditor5-table/src/ui/utils.js
index 5e2a4e3bfb..32263b5093 100644
--- a/packages/ckeditor5-table/src/ui/utils.js
+++ b/packages/ckeditor5-table/src/ui/utils.js
@@ -16,6 +16,7 @@ import { isColor, isLength, isPercentage } from '@ckeditor/ckeditor5-engine/src/
 import { getTableWidgetAncestor } from '../utils';
 import { findAncestor } from '../commands/utils';
 import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';

 const DEFAULT_BALLOON_POSITIONS = BalloonPanelView.defaultPositions;
 const BALLOON_POSITIONS = [
@@ -26,6 +27,29 @@ const BALLOON_POSITIONS = [
          DEFAULT_BALLOON_POSITIONS.southArrowNorthWest,
          DEFAULT_BALLOON_POSITIONS.southArrowNorthEast
 ];
+const TABLE_PROPERTRIES_BALLOON_POSITIONS = [
+         ...BALLOON_POSITIONS,
+         ( tableRect, balloonRect ) => {
+                  const viewportRect = new Rect( global.window );
+                  const isUpperTableEdgeBeyondViewport = tableRect.top < viewportRect.top;
+                  const isLowerTableEdgeBeyondViewport = tableRect.bottom > viewportRect.bottom;
+                  let top, name;
+
+                  if ( isUpperTableEdgeBeyondViewport && !isLowerTableEdgeBeyondViewport ) {
+                           top = Math.min( tableRect.bottom, viewportRect.bottom ) - balloonRect.height - BalloonPanelView.arrowVerticalOffset;
+                           name = 'arrow_s';
+                  } else {
+                           top = BalloonPanelView.arrowVerticalOffset + Math.max( tableRect.top, 0 );
+                           name = 'arrow_n';
+                  }
+
+                  return {
+                           top,
+                           left: tableRect.left + tableRect.width / 2 - balloonRect.width / 2,
@@ -16,6 +16,7 @@ import { isColor, isLength, isPercentage } from '@ckeditor/ckeditor5-engine/src/
 import { getTableWidgetAncestor } from '../utils';
 import { findAncestor } from '../commands/utils';
 import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';

 const DEFAULT_BALLOON_POSITIONS = BalloonPanelView.defaultPositions;
 const BALLOON_POSITIONS = [
@@ -26,6 +27,29 @@ const BALLOON_POSITIONS = [
          DEFAULT_BALLOON_POSITIONS.southArrowNorthWest,
          DEFAULT_BALLOON_POSITIONS.southArrowNorthEast
 ];
+const TABLE_PROPERTRIES_BALLOON_POSITIONS = [
+         ...BALLOON_POSITIONS,
+         ( tableRect, balloonRect ) => {
+                  const viewportRect = new Rect( global.window );
+                  const isUpperTableEdgeBeyondViewport = tableRect.top < viewportRect.top;
+                  const isLowerTableEdgeBeyondViewport = tableRect.bottom > viewportRect.bottom;
+                  let top, name;
+
+                  if ( isUpperTableEdgeBeyondViewport && !isLowerTableEdgeBeyondViewport ) {
+                           top = Math.min( tableRect.bottom, viewportRect.bottom ) - balloonRect.height - BalloonPanelView.arrowVerticalOffset;
+                           name = 'arrow_s';
+                  } else {
+                           top = BalloonPanelView.arrowVerticalOffset + Math.max( tableRect.top, 0 );
+                           name = 'arrow_n';
+                  }
+
+                  return {
+                           top,
+                           left: tableRect.left + tableRect.width / 2 - balloonRect.width / 2,
+                           name
+                  };
+         }
+];

 const isEmpty = val => val === '';

@@ -69,7 +93,7 @@ export function getBalloonTablePositionData( editor ) {

          return {
                   target: editor.editing.view.domConverter.viewToDom( viewTable ),
-                  positions: BALLOON_POSITIONS
+                  positions: TABLE_PROPERTRIES_BALLOON_POSITIONS
          };
 }

Just to make things clear: using the current API, table tools have no way of knowing where the global editor toolbar is and how it behaves so without a massive refactoring there is no way to avoid the overlapping UIs. 

If you like this approach, I'd also adopt it for the table toolbar which UX suffers the same issue.

niegowski added a commit that referenced this issue May 15, 2020
Fix (widget): The widget toolbar should always be visible even if the widget is longer than a visible viewport (see #6190).

Fix (table): The table properties balloon should always be visible if the table is longer than a visible viewport. Closes #6190.
@Reinmar Reinmar added the domain:accessibility This issue reports an accessibility problem. label Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:accessibility This issue reports an accessibility problem. domain:ui/ux This issue reports a problem related to UI or UX. package:table type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
3 participants