User tests: Successful: Unsuccessful:
#33466 @bembelimen @richard67 @joomdonation
Fulfils these new requirements from others
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_categories com_tags Libraries |
Labels |
Added:
?
|
But do they work :)
Adding release blocker label as inherited from the issue.
This isnt a release blocker. If this matches the criteria to block a release then the tag should be applied to 90% of pr
Well I don't really care. It will be merged in time anyway if it's good. Will see if I can test tomorrow.
Could you please move reset states logic to a separate PR? My feeling is that getIsEmptyState
should not do the reset states itself. It is up to caller to do that if it wants. Further more, I'm unsure if the implementation of resetState is right but that is a different discussion
For this one, could we please focus on fixing the empty state logic? One problem at a time.
So once again one person wants it and another person doesn't want it. I just can't win. It works, And has zero knock on effects.
So once again one person wants it and another person doesn't want it. I just can't win. It works, And has zero knock on effects.
I just express my concern and want to move it to separate PR so that maintainers and review and make decision easier. At the end, you don't need to make everyone happy. If you think it is OK, just leave it in this PR and maintainers will decide.
So I still believe (myself) that resetState
should be implemented like that or is called within getIsEmptyState
(maybe it is just me, don't worry). As I said, you don't have to make everyone happy. If you want to keep it, leave it as how it is. Other maintainers will review and make decision.
In case you agree with me, I made a PR to your branch PhilETaylor#7 which I think it will make this PR safe to merge. Please take a look at it (especially the change in Categories model which we discussed earlier in this PR) and if it works and you are happy, merge it.
For me it's ok to remove the resetState here and do it with another PR, to come forward here. I don't even insist on my issue to be fixed, I just thought it could be a good idea.
I have just landed and will work on this today. Again. But the pr proposed by @joomdonation i think has a major flaw - but I will be verbose in explaining that when I'm next at a desk and not sat at the controls of an airplane flying
I take it back. The concern I had was unfounded as using _getListCount
takes my concern (about joined tables like tags still being left in the select, when they were removed as a join). _getListCount
does this clearing of select
and readds select(*)
for us,
I have merged your PR to my branch.
I have removed limit and offset from being cleared in getEmptyStateQuery
- these are removed by _getListCount anyway
Once this is merged I'll do a quick recap of all the issues and outstanding states in #33266 and then I'll be stepping back and taking a break. Thanks for your support on these, I think Joomla is better for this new feature and only sad the rest of my work has not made it as far.
I have tested this item
@PhilETaylor Thanks for working so hard for Joomla! these days.
I've updated the branch to latest 4.0-dev to get rid of the unrelated javascript-cs error in drone and restored the previous test result in the issue tracker.
I have tested this item
I tested all views with empty states and checked using search and filters. I couldn't find problems. And the problem with "disappearing" categories when using filter tag is gone.
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-05-02 19:30:46 |
Closed_By | ⇒ | bembelimen | |
Labels |
Added:
?
|
Thank you!
Thank you for the improvements.