Skip to content

Comments

[4.0] Admin modules: Cannot select or filter by position#23289

Merged
wilsonge merged 12 commits intojoomla:4.0-devfrom
infograf768:4-0.admin_modules_position
Jan 4, 2019
Merged

[4.0] Admin modules: Cannot select or filter by position#23289
wilsonge merged 12 commits intojoomla:4.0-devfrom
infograf768:4-0.admin_modules_position

Conversation

@infograf768
Copy link
Member

#22263 forgot admin modules

Summary of Changes

Change type of the position field for admin modules.
Setting the correct client id.

Testing Instructions

Install j4 from 4.0-dev or nightly.
Display Administrator Modules Manager
Create or edit one these modules.

Note: there are other errors

Expected result

The Position field should be similar to the one used for Site Modules, i.e. letting clear, select, set to None the position.
Filtering by position should display the administrator template positions

Actual result

For the manager, positions are sometimes the ones from the frontend template in the filter
screen shot 2018-12-16 at 17 36 51

Editing the admin module displays a simple field
screen shot 2018-12-16 at 17 37 09

After patch

screen shot 2018-12-16 at 17 26 51

screen shot 2018-12-16 at 17 27 13

Note

There are other errors, not only related to admin modules.

  1. The calendar icons are showing over the position dropdown (see image above)
  2. After filtering in the manager by anything else than Select and clicking anywhere on the page, the filter fields are displayed horizontally on the page, hiding some items.

screen shot 2018-12-16 at 17 40 57

General Comment

The decision to take off from the Modules Manager the Site/Admin filter is at the origin of some issues concerning userstate and filters specially when a user switches from one to the other via the menu or System CPanel.

@Fedik

// In case of admin module
if ($this->item->client_id == 1)
{
$input->set('client_id', 1);
Copy link
Member

Choose a reason for hiding this comment

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

Input shouldn't be modified in a layout file.

Copy link
Member Author

Choose a reason for hiding this comment

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

@laoneo
What do you suggest to solve the issue?

Copy link
Member Author

@infograf768 infograf768 Dec 17, 2018

Choose a reason for hiding this comment

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

The other solution is to create specific fields for admin modules.
I could not find another way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Another solution: doing that in the models.
On it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Please check.

Copy link
Member

Choose a reason for hiding this comment

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

I think a correct way would be to use a field attribute.
but for this also need to improve ModulesPositioneditField also

Copy link
Member Author

Choose a reason for hiding this comment

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

see below

@infograf768 infograf768 force-pushed the 4-0.admin_modules_position branch from 9da4b8e to 9afd992 Compare December 17, 2018 11:18
@SharkyKZ
Copy link
Contributor

SharkyKZ commented Dec 17, 2018

Is this not the same as #22040? At least the filter part.

@infograf768
Copy link
Member Author

not really as we have a new field and the modulesposition field was modified in 4.0.
see
#22263

@Fedik
Copy link
Member

Fedik commented Dec 17, 2018

The calendar icons are showing over the position dropdown

that is CSS issue, somewhere, I tried to fix it in https://github.com/joomla/joomla-cms/pull/22263/files#diff-712f8768cba688c1b024f23f9941f51eR4 not sure why it there again

@infograf768
Copy link
Member Author

@SharkyKZ
You are right. Once again one of these aspects of 4.0 which is way behind staging.
What shall we do?
What is the best code?
If it is the one you used for 3.9.0, I can modify this patch, but I also would have to apply the changes from your PR here for all to work OK.

@wilsonge

@infograf768
Copy link
Member Author

@Fedik
If we use the code in 3.9.0 from @SharkyKZ then indeed we just have to modify the way to get the $clientId in ModulesPositioneditField.php by using getUserState('com_modules.modules.client_id', 0, 'int')

Concerning the dropdown z-index, it looks like .choices__list--dropdown in template.css is loaded BEFORE the /media/vendor/choicesjs/css/choices.css and is therefore overriden by that file. (Debug on).

So we get

.choices__list--dropdown {
  display: none;
  z-index: 1;
  position: absolute;
  width: 100%;
  background-color: #FFFFFF;
  border: 1px solid #DDDDDD;
  top: 100%;
  margin-top: -1px;
  border-bottom-left-radius: 2.5px;
  border-bottom-right-radius: 2.5px;
  overflow: hidden;
  word-break: break-all;
}

and z-index = 1; instead of 10

@SharkyKZ
Copy link
Contributor

That should work. Just don't add 3rd argument to getUserState(). I added it by mistake. Oops.

@infograf768 infograf768 force-pushed the 4-0.admin_modules_position branch from 46a87f0 to cc8f54b Compare December 18, 2018 07:03
@infograf768
Copy link
Member Author

Modified to use getUserState
Please test.

@ReLater
Copy link
Contributor

ReLater commented Dec 30, 2018

I have tested this item ✅ successfully on c4d5e9e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23289.

@anibalsanchez
Copy link
Contributor

I have tested this item ✅ successfully on 76b957b

Test OK


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23289.

@infograf768
Copy link
Member Author

RTC. Thanks for testing.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23289.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 4, 2019
@infograf768 infograf768 added this to the Joomla 4.0 milestone Jan 4, 2019
@wilsonge wilsonge merged commit 80c497f into joomla:4.0-dev Jan 4, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Jan 4, 2019

Thanks guys!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 4, 2019
@infograf768 infograf768 deleted the 4-0.admin_modules_position branch January 5, 2019 06:53
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

Successfully merging this pull request may close these issues.

9 participants