Problem/Motivation

Tabs should be hidden if there is nothing to display in the Behavior perspective.

Proposed resolution

Remove tabs from the code if there are no active behavior plugins.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

drobnjak created an issue. See original summary.

toncic’s picture

Assigned: Unassigned » toncic

I will do this.

drobnjak’s picture

Assigned: toncic » drobnjak

As it is follow-up on perspectives, I will continue with this

miro_dietiker’s picture

This could be achievedd by creating the tabs in js and only do so conditionally.

miro_dietiker’s picture

Priority: Normal » Critical

Promoting since the ux sucks for cases without plugins.

drobnjak’s picture

Status: Active » Needs review
StatusFileSize
new816 bytes

Checking the number of behavior elements and hiding tabs if there are no behavior behavior elements. We are not moving the tabs creation in the front-end, since selectors are created in the back-end, and they would have to be sent as parameters in Drupal settings. Therefore, there is no way to completely move it to the front-end, and we should do it with lightweight JS check.

miro_dietiker’s picture

+++ b/js/paragraphs.admin.js
@@ -29,6 +29,15 @@
+        $(context).find('.paragraphs-tabs-wrapper').find('.paragraphs-tabs').hide();
...
+        $(context).find('.paragraphs-tabs-wrapper').find('.paragraphs-tabs').show();

Additionally, i would think that by default CSS should hide it.

We need the show and hide in JS to respond to added / removed paragraphs?

In any case, i think the user journey hiding / showing the tabs should be described with test coverage.

miro_dietiker’s picture

Status: Needs review » Needs work
drobnjak’s picture

Status: Needs work » Needs review
StatusFileSize
new1021 bytes
new1.1 KB

Setting default display for tabs to none. This automatically solves the no-JS fallback and we are displaying tabs only if there are behavior elements.

miro_dietiker’s picture

+++ b/js/paragraphs.admin.js
@@ -29,6 +29,12 @@
+      if(document.getElementsByClassName('paragraphs-behavior').length != 0) {

We have jQuery. So why not $('.paragraphs-behavior')

drobnjak’s picture

StatusFileSize
new991 bytes
new608 bytes

Addressing comment #11

drobnjak’s picture

primsi’s picture

Status: Needs review » Needs work

IMHO we should do it the other way around, show the tab in css but hide it in js. I think that we should be able to test this with js tests if I am not mistaken.

miro_dietiker’s picture

Status: Needs work » Needs review

@Primsi: The tabs should not be visible in NoJS. Also they should not be temporarily visible if the widget has no behavior tab.
I think the logic is correct to only add the tabs / make them visible if we detect a need for it.

miro_dietiker’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/js/paragraphs.admin.js
@@ -29,6 +29,12 @@
+        $(context).find('.paragraphs-tabs-wrapper').find('.paragraphs-tabs').show();

Oh you dropped the hide() method here that was present before.
Unsure, but: Say a user removes all paragraphs and there are no behavior paragraphs remaining anymore, the tabs now still remains? Before it was removed again.

We need JS tests that proof the tabs are not present if there are is no behavior element.

drobnjak’s picture

Status: Needs work » Needs review
StatusFileSize
new3.2 KB
new2.46 KB

Adding back .hide() method I removed for the case there are no behavior paragraphs. Expanded tests with method to check tabs visibility if there are no behavior elements. Since there is a lot of duplicated code in tests, most of it will be moved into traits in this issue #2867049: Extend JS tests for edit perspectives

Status: Needs review » Needs work

The last submitted patch, 17: only_display_tabs_if-2862935-17.patch, failed testing.

drobnjak’s picture

Status: Needs work » Needs review

  • Primsi committed d36117e on 8.x-1.x authored by drobnjak
    Issue #2862935 by drobnjak, miro_dietiker: Only display tabs if there is...
primsi’s picture

Status: Needs review » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.