[#11899] - [com_newsfeeds] - "Filter field" in News Feeds - list layout
- Fixed in Code Base
- 24 Jan 2017
- Medium
- Build: staging
- Easy Test
- # 11899
- Diff
- alikon:patch-79
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
| Title |
|
||||||
joomla-cms-bot
- | Category | ⇒ | Front End Components |
| Labels |
Added:
?
|
||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
I have tested this item
I can confirm, now ok also for "table headings" layout.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.
I have tested this item
Thanks alikon
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.
| Labels |
Added:
?
|
||
| Milestone |
Added: |
||
| Labels |
Removed:
?
|
||
@joomla-bot: to RTC or not to RTC?
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.
| Labels |
Added:
?
|
||
| Labels |
Removed:
?
|
||
@joomla-cms-bot why do you hate RTC on this PR ?
@jeckodevelopment can you try again ?
@jeckodevelopment - its already under investigation by @mbabker
On 6 September 2016 at 09:02, Luca Marzo notifications@github.com wrote:
@alikon https://github.com/alikon at the moment is impossible to change
the status of this PR via issues.joomla.org
@mbabker https://github.com/mbabker can you look at this?—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11899 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8d-ZJVqW_NAs5nL24tI8HWyHlVpOks5qnR4lgaJpZM4J0Ci0
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Follow the issues on the issue tracker instead of pinging every time something fails please. You'll note in there there are open issues and open tickets elsewhere to try and address things.
| Status | New | ⇒ | Ready to Commit |
| Labels |
Added:
?
|
||
| Milestone |
Removed: |
||
| Status | Ready to Commit | ⇒ | Needs Review |
| Labels | |||
| Milestone |
Removed: |
||
I am not going to merge this one:
- I think it is wrong to have two total different markups when changing a parameter
- This here seems to do more then fixing the mentioned bug
- Checking ACL in the view seems to me wrong, that's application logic.
Removing RTC set to needs review
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.
| Labels |
Removed:
?
|
||
Sorry I should have checked the code.
I'm now wondering why there is an additional view to achieve this. We dont need to do that elsewhere
I think it is wrong to have two total different markups when changing a parameter
yes but when this parameter is set you should use a table markup instead of <ul><li>...</li></ul>
and the table markup was missed, so i've used 2 different to mantain the previous behaviour as it was and keep the things simply separated , if is better to have only one with mixed things, then ok ....
This here seems to do more then fixing the mentioned bug
the table heading was totally uncoded so i've added it (maybe in the wrong way ?)
Checking ACL in the view seems to me wrong, that's application logic.
i don't want to justify myself, i'm always open to improve myself, but cause i'm lazy, i've simply "cloned"
what we already do elesewhere https://github.com/joomla/joomla-cms/blob/staging/components/com_content/views/article/tmpl/default.php#L143
p.s.
now i can understand why the joomla-cms-bot was reluctant to set this to rtc
now i can understand why the joomla-cms-bot was reluctant to set this to
rct?
lol
@rdeutz @alikon is this PR being changed?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
I think my comment is still valid
| Title |
|
||||||
i've changed this PR
now it solve only the bug with "Filter field"
@rdeutz is this ok now?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
seems fine now
@rdeutz and @brianteeman should we test it again?
| Status | Needs Review | ⇒ | Pending |
| Easy | No | ⇒ | Yes |
Set back to pending for NEW tests
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
| Labels |
Removed:
?
|
||
| Category | Front End Components | ⇒ | Front End com_newsfeeds Components |
I have tested this item
Bug with not hiding Filter field is solved.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
@alikon @franz-wohlkoenig i have just fixed the merge conflicts please review and give it a quick retest. Thanks!
ok4me
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11899.
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-01-24 08:18:46 |
| Closed_By | ⇒ | rdeutz |
I have tested this item✅ successfully on 30b41ea
Ciao Alikon,
fix ok for the "Filter field" but no for "Table Headings"
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11899.