The problem with this approach (which has been tried before) is that you can not trash an item when you have the filter set to all

The problem with this approach (which has been tried before) is that you can not trash an item when you have the filter set to all

I understand. FYI this approach is already used in other components view.

Example for contacts

			if ($this->state->get('filter.published') != -2)
			{
				$childBar->trash('contacts.trash')->listCheck(true);
			}

for categories

			if ($canDo->get('core.edit.state') && $this->state->get('filter.published') != -2)
			{
				$childBar->trash('categories.trash')->listCheck(true);
			}

One solution could be to change each conditional to something like

			if ($canDo->get('core.edit.state') && ($this->state->get('filter.published') != -2 || !$this->state->get('filter.published') === '*'))
			{
				$childBar->trash('categories.trash')->listCheck(true);
			}

In this case Trash will display also when status is set to All

@gostn
com_fields is used for all components
articles, contacts, users

question was, if all component-views are meant to test.

Not necessary. One is enough.
But anyway, a decision has to be taken concerning #31074 (comment) which would modify this patch

@HLeithner @wilsonge

Joomla always had it that way, make it as hard as possible to delete an item...

I'm not sure if we should change this...

The decision here is simple:
or we forget this patch and make a new one for the items where hiding the Trash dropdown is already implemented and kill this behavior (examples above), or we follow up on this patch and add code to display the trash only when the list is not filtered by trashed or filtered by all.

Show the trash only when the filter is on "trashed" and remove the separate "trash" button

The separate button is Empty Trash, not Trash.
We can, in an other PR, move it to the dropdown and change anyway its wording to Delete.

In the mean while, I understand that this PR is fine and we are not going to consider filtering by All as necessitating the display of Trash in the dropdown.

@gostn
I guess you can test now :)

@infograf768 can you update test instructions?

What shall I update?

Done

@gostn
#31074 (comment)

I had a discussion with @HLeithner and we decided filtering by All did not necessitate having Trash as a choice in the dropdown.

It means this PR is ready for testing.

I had a discussion with @HLeithner and we decided filtering by All did not necessitate having Trash as a choice in the dropdown.

That's disappointing

The filter on trashed state is a "human accident" protection (like the complete trash concept).

The concept to change the state to trashed is a ux fail in my opinion and should/could be optimised with a better ui, maybe a button "show trash" which basically set only the filter.

We can came back to to "delete" on all filter " when we change the action drop down to a more dynamic list based on the state of the selexted items.

I have tested this item ✅ successfully on 21c4650

Done

@gostn

Without PR in ´News Feed > Feed´ theres no ´Trash´, its ´Empty Trash´, same with PR:

This is totally unrelated to this patch.
The matter of normalising the "Empty Trash" button by

1. Changing its wording to "Delete"
2. Displaying it in the Actions dropdown when filtering by trashed items

Is a separate issue.

Once this PR is merged, i.e when we, at last, get a second positive test, merge and move along, we can discuss and create another PR concerning this matter.

What some could consider as an issue is the length of the dropdown in some cases (yes, I already have a PR ready):

Newsfeeds is taken care of in this PR concerning hiding the Trash action in the Actions dropdown when filtering by Trashed items.
The Empty trash being displayed in the Actions dropped down is not modified here. It is a different behavior than other components indeed where we have a separate button and is a separate matter...

You are right. It was already implemented before this patch.
Here I just added some code that was useless as $canDo->get('core.edit.state') was already there.

modifying now as the feature was present for newsfeeds.

Modified to take off redundant check for newsfeeds.
Modified test instructions.

I have tested this item ✅ successfully on 928a680

Before and after, it looks like the screen grabs in the testing instructions, when testing the patch, thanks @infograf768

Before, screen grab...

After, screen grab...

RTC. Thanks for testing!

This should not have been merged as it causes a major loss in functionality