? Language Change Maintainers Checked PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
13 Jul 2022

This is an update of the abandoned PR #32671 with conflicts fixed and codestyle updated.

Summary of Changes

See the original PR for further details and testing instructions

avatar brianteeman brianteeman - open - 13 Jul 2022
avatar brianteeman brianteeman - change - 13 Jul 2022
Status New Pending
avatar brianteeman brianteeman - change - 13 Jul 2022
Labels Added: Language Change ?
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2022
Category Administration com_banners com_contact com_content com_languages com_newsfeeds Language & Strings Modules
3d4166c 13 Jul 2022 avatar brianteeman order
avatar Kostelano
Kostelano - comment - 13 Jul 2022

What is the principle of sorting in the module parameters and directly in the panel? Very chaotic.

Screenshot_2

Screenshot_1

avatar brianteeman
brianteeman - comment - 14 Jul 2022

What is the principle of sorting in the module parameters and directly in the panel? Very chaotic.

Agreed. I couldnt see any logic in the original PR.

Happy to look at any suggestions - but not alphabetical

avatar chmst
chmst - comment - 18 Jul 2022

@Kostelano there is no logic. And as every site is individual and need some buttons but does not need other buttons, finding a structure is not as easy. I cannot imagine a site with all buttons activated.
For example:
I don't need users because I have no user registration.
I have all plugins installed and used the plugins button only during development (same for template buttons).
During development I don't need a cache or checkin function.

So we cannot make a structure here.
Maybe "used in development only", or "buttons with count"?

I still think that the wohle feature needs complete re-writing with individual ordering and individual classes but meanwhile it is an improvment if there are more icons available.

avatar Kostelano
Kostelano - comment - 18 Jul 2022

I agree that "the only correct structure" is simply impossible to find, but there are some common things to understand - for example, that featured articles should be somewhere near regular articles, etc.

I played a little in photoshop. I don’t pretend to be the truth, but, let’s say, such a strauture (namely, the SEQUENCE of points, taking into account the possible disabling of some points or the FULL inclusion of all points, which is actually unlikely) would completely suit me. Although in the process of drawing, I moved points back and forth several times when I did not understand what I wanted to see.

Current view:

Screenshot_1

My suggestion:

Screenshot_1 — копия

Please, I ask visitors to take part in some kind of feedback, because in my opinion it is an interesting and necessary PR. Thank you.

avatar brianteeman
brianteeman - comment - 18 Jul 2022

I still think that the wohle feature needs complete re-writing with individual ordering and individual classes but meanwhile it is an improvment if there are more icons available.

That is exactly my thought on this. Merge this now and then rewrite it with ordering later.

avatar Fedik
Fedik - comment - 18 Jul 2022

It also can be a subform, with dropdown which "thing" to show. Then everyone can do their own ordering :)
But that need much changes in PR.

avatar chmst
chmst - comment - 18 Jul 2022

@Kostelano I like your ordering.

avatar Kostelano
Kostelano - comment - 18 Jul 2022

@brianteeman If you support my proposal to change the order of elements, I am ready to make a change in your branch. I just don't want to waste my time ;)

avatar brianteeman
brianteeman - comment - 18 Jul 2022

I think you will find its not as simple as you think it will be

avatar chmst
chmst - comment - 19 Jul 2022

If we can get two success tests, we can merge this for now and then you can make a PR for ordering @Kostelano

avatar Kostelano
Kostelano - comment - 19 Jul 2022

I have tested this item successfully on 26f4aab


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

avatar Kostelano Kostelano - test_item - 19 Jul 2022 - Tested successfully
avatar chmst
chmst - comment - 19 Jul 2022

I have tested this item successfully on 26f4aab


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

avatar chmst chmst - test_item - 19 Jul 2022 - Tested successfully
avatar chmst chmst - change - 19 Jul 2022
Status Pending Ready to Commit
avatar chmst
chmst - comment - 19 Jul 2022

RTC


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

avatar chmst
chmst - comment - 20 Jul 2022

@brianteeman there are unresolved conversations and block merging. Could you please check?

avatar brianteeman brianteeman - change - 20 Jul 2022
Labels Added: ? Maintainers Checked
avatar brianteeman
brianteeman - comment - 20 Jul 2022

@chmst stupid github didnt close the conversations when the code was corrected

avatar fancyFranci
fancyFranci - comment - 7 Sep 2022

It is a really nice PR, but I have to say that 4.3 is a better place for these improvements. I will change the base branch. Thank you for finishing it!

avatar brianteeman
brianteeman - comment - 7 Sep 2022

at the time the code was written and made rtc 4.2 was the correct place

avatar obuisard obuisard - change - 7 Sep 2022
Labels Added: PR-4.3-dev
Removed: ?
avatar obuisard
obuisard - comment - 8 Sep 2022

I like this PR, it may add clutter to the interface, but that should be at the discretion of the administrator to do what it's right for the website managed. It is really nice to be able to have a full toolbox of all possible shortcuts.

We will need to complement this PR with 2 other ones:

  • allow for the ordering of the selected shortcuts,
    - replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.
avatar brianteeman
brianteeman - comment - 9 Sep 2022

allow for the ordering of the selected shortcuts,

good luck with that

avatar chmst
chmst - comment - 9 Sep 2022

@obuisard Ordering is a big task and not in scope of this PR. It would need a complete rework / another concept.

avatar brianteeman brianteeman - change - 9 Sep 2022
Title
[4.2] Quickicons
[4.3] Quickicons
avatar brianteeman brianteeman - edited - 9 Sep 2022
avatar obuisard
obuisard - comment - 9 Sep 2022

@obuisard Ordering is a big task and not in scope of this PR. It would need a complete rework / another concept.

Indeed, that is why we need to create 2 other PRs to improve on this one :-)
And I agree, the ordering of the quickicons is challenging

avatar brianteeman
brianteeman - comment - 9 Sep 2022

until this is merged no one can even try

avatar N6REJ
N6REJ - comment - 9 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

avatar obuisard
obuisard - comment - 9 Sep 2022

until this is merged no one can even try

I won't blindly merge PRs.

avatar obuisard
obuisard - comment - 9 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

Is it @N6REJ ? because I don't see the fa fa-xxx icons on my 4.2 version

avatar brianteeman
brianteeman - comment - 9 Sep 2022

until this is merged no one can even try

I won't blindly merge PRs.

would object if you did

avatar N6REJ
N6REJ - comment - 10 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

Is it @N6REJ ? because I don't see the fa fa-xxx icons on my 4.2 version

There were 3 large pr's that added fa- to every J! icon. At that time they were the "We're using fontawesome from now on" icon set. but we were told for b/c we had to also provide ability for standard icon-
Since then everything has been moved to a js build so I've no clue where everything went nor what happened.

avatar obuisard
obuisard - comment - 12 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

For reference, the change to -icons was made October 2020 to remove the dependency on FontAwesome. #30707

avatar brianteeman
brianteeman - comment - 12 Sep 2022

this pr is already using icon- notation so I don't even know why it is being discussed.

avatar brianteeman
brianteeman - comment - 12 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

Is it @N6REJ ? because I don't see the fa fa-xxx icons on my 4.2 version

There were 3 large pr's that added fa- to every J! icon. At that time they were the "We're using fontawesome from now on" icon set. but we were told for b/c we had to also provide ability for standard icon- Since then everything has been moved to a js build so I've no clue where everything went nor what happened.

I think you are confused. The PRs you created were not merged https://github.com/joomla/joomla-cms/pulls?q=is%3Apr+author%3AN6REJ+is%3Aclosed+icon+is%3Aunmerged+

avatar N6REJ
N6REJ - comment - 13 Sep 2022

Sorry @brianteeman but your VERY wrong... lets start with a simple one #27686

avatar N6REJ
N6REJ - comment - 13 Sep 2022

@brianteeman are you introducing any icons that don't pre-exist in the fa- compatibility table?

avatar brianteeman
brianteeman - comment - 13 Sep 2022

Read the code and test the pr and stop wasting everyone's time. This bike shedding is why it takes so long to move forward.

avatar N6REJ
N6REJ - comment - 13 Sep 2022
  • replace the old icon classes "icon-xxx" with "fa fa-xxx" in the quickicon module to match the new way icons are implemented in Joomla 4 - this is not something introduced in this PR, it is already something that needs to be fixed in the official release as well.

That was done last year or so but this pr seems to be undoing it.

Is it @N6REJ ? because I don't see the fa fa-xxx icons on my 4.2 version

just an fyi, per our conversation, you can still ( as of 4.2.2) use fa- instead of icon- in admin if you desire.
image

avatar obuisard obuisard - change - 17 Sep 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-09-17 01:43:44
Closed_By obuisard
avatar obuisard obuisard - close - 17 Sep 2022
avatar obuisard obuisard - merge - 17 Sep 2022
avatar obuisard
obuisard - comment - 17 Sep 2022

Thank you very much Brian @brianteeman for the PR and all who participated in the conversation. Thank you @Bongovo for the original PR.

Add a Comment

Login with GitHub to post a comment