Skip to content

Comments

[5.0] Fix, Hide toolbar in modal when it not in use#41578

Closed
Fedik wants to merge 10 commits intojoomla:5.0-devfrom
Fedik:modal-toolbar
Closed

[5.0] Fix, Hide toolbar in modal when it not in use#41578
Fedik wants to merge 10 commits intojoomla:5.0-devfrom
Fedik:modal-toolbar

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Sep 4, 2023

Pull Request for Issue #41572 .

Summary of Changes

Hide toolbar on empty state, in modal.
Because $this->setLayout('emptystate'); override the layout value the condition if ($this->getLayout() !== 'modal') never work.

Testing Instructions

On installation without articles.
Try select an article for menu. Also Category, Contact, Tags

Also try to add a module to dashboard.

Actual result BEFORE applying this Pull Request

You get a toolbar in modal

Expected result AFTER applying this Pull Request

No toolbar

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:
  • No documentation changes for docs.joomla.org needed
  • Pull Request link for manual.joomla.org:
  • No documentation changes for manual.joomla.org needed

Reference:

@richard67 richard67 added the bug label Sep 4, 2023
@richard67
Copy link
Member

I have tested this item ✅ successfully on 63b034b


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

1 similar comment
@heelc29
Copy link
Contributor

heelc29 commented Sep 4, 2023

I have tested this item ✅ successfully on 63b034b


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

@richard67 richard67 removed the bug label Sep 4, 2023
@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 4, 2023
@richard67 richard67 added the bug label Sep 4, 2023
@Fedik
Copy link
Member Author

Fedik commented Sep 4, 2023

Wait, @obuisard found double toolbar in media modal.

@Fedik
Copy link
Member Author

Fedik commented Sep 4, 2023

Should be good now also for Media.
Can only re-test "image select" while aricle editing

@richard67
Copy link
Member

Back to pending.


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

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 4, 2023
@obuisard
Copy link
Contributor

obuisard commented Sep 4, 2023

Fedir @Fedik the media modal is now working properly

@obuisard
Copy link
Contributor

obuisard commented Sep 4, 2023

Got 3 different results while testing after an update, trying now on new install (new install fails with Class "Joomla\Plugin\Behaviour\Taggable\Extension\Taggable" not found so updating the branch).

image

@Fedik
Copy link
Member Author

Fedik commented Sep 4, 2023

Got 3 different results while testing after an update

✅ "New article" uses new ModalField, so it is okay ,
✅ "New contact" uses old ModalField, so it also okay ,
❌ "New newsfeed" uses old ModalField, and it need to fix the toolbar,

@obuisard
Copy link
Contributor

obuisard commented Sep 4, 2023

Got 3 different results while testing after an update

✅ "New article" uses new ModalField, so it is okay , ✅ "New contact" uses old ModalField, so it also okay , ❌ "New newsfeed" uses old ModalField, and it need to fix the toolbar,

I confirm. Also
✅ "New categories"

Could not find where tags could be created in modal.

@Fedik
Copy link
Member Author

Fedik commented Sep 5, 2023

newsfeed should be good now

}

$this->addToolbar();
if ($this->getLayout() !== 'modal') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to nitpick. In the other files, you assigned it to $isModal.

Copy link
Member Author

Choose a reason for hiding this comment

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

it need there, because emptystate layout, and here it not in use

@Fedik
Copy link
Member Author

Fedik commented Sep 5, 2023

I will set it as Draft for now.
There other related issues, wich need to find a better approach.

@Fedik Fedik marked this pull request as draft September 5, 2023 10:37
@Fedik Fedik changed the title [5.0] Fix #41572, Hide toolbar on empty state, in modal [5.0] Fix, Hide toolbar in modal when it not in use Sep 5, 2023
@Fedik
Copy link
Member Author

Fedik commented Sep 5, 2023

Please test alternative fix #41600

@Fedik Fedik closed this Sep 5, 2023
@Fedik Fedik deleted the modal-toolbar branch September 5, 2023 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants