Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate inline help toggle button to com_config component Options view (see #35610) #37158

Merged
merged 3 commits into from
Mar 3, 2022

Conversation

cyrezdev
Copy link
Contributor

@cyrezdev cyrezdev commented Feb 27, 2022

Pull Request to extend PR #35610.
cc/ @nikosdion

Summary of Changes

  • Integrate inline help (toggle button) feature to com_config component Options view
  • [com_content] : show inline help toggle button
  • Other components will be updated if it's ok with this PR the way i did it. ;-)

Testing Instructions

  1. Apply Patch
  2. Go to Articles component
  3. Open Options
  4. Inline help text hidden
  5. Click on the toggle inline help button on top right to show / hide inline help.

Actual result BEFORE applying this Pull Request

No inline help toggle button in com_content options.

Expected result AFTER applying this Pull Request

Toggle button to show/hide inline help in com_content options.

Documentation Changes Required

To integrate inline help toggle button in any component (core + third party), add the following line in your config.xml file for components. (after element)
<inlinehelp button="show" targetclass=""/>

Params:

  • button : 'show' to enable the toggle button for inline help text.
  • targetclass : the class of the DIVs to toggle visibility on.

@cyrezdev cyrezdev changed the title Integrate inline help feature to com_config component Options view (see #35610) Integrate inline help toggle button to com_config component Options view (see #35610) Feb 28, 2022
@prakhar3062
Copy link
Contributor

branch is out of date so not able to test it

@alikon
Copy link
Contributor

alikon commented Feb 28, 2022

you can use the Prebuilt package

@nikosdion
Copy link
Contributor

I have tested this item ✅ successfully on 80c2f8a

I am 100% onboard with this change. It will allow 3PDs to use this feature as well (which is something I am personally really looking forward to!).

As for com_content and other components, we will need to go through their Options pages with a fine comb to see which options need to be documented and how. But as you said that't the subject of another PR :)


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

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Mar 1, 2022

It will allow 3PDs to use this feature as well (which is something I am personally really looking forward to!).

Testing with my own extension, and working perfectly (even including your inlinehelp button in my edit view works well!)
I have maybe one suggestion: possibility to set by default visibility on show or hide (currently only hidden by default).
Storing in user session preference could be a bonus too.
This is a very useful feature, and allow better options documentation without throwing unnecessary text for advanced users.
Good work! 👍

As for com_content and other components, we will need to go through their Options pages with a fine comb to see which options need to be documented and how. But as you said that't the subject of another PR :)

So, should i enable it in all core components in this PR, or wait inlinehelp review for each component, and enable option when review is complete ?

@nikosdion
Copy link
Contributor

@cyrezdev Regarding this:

possibility to set by default visibility on show or hide (currently only hidden by default).
Storing in user session preference could be a bonus too.

If you read the original PR (#35610), namely the “Why make this change” you will see why this is not going to happen. You already figure it out yourself.

This is a very useful feature, and allow better options documentation without throwing unnecessary text for advanced users.

Yup. That's why.

Ideally, we'd need real user testing to know if the assumption that beginners will click that button is true or if we need to revise. Fat chance of that happening.

The next best thing I could do is deploy support of Show Inline Help to my security software which has an infamously extensive configuration page. The past ~9 years it had tooltips. Now it has Show Inline Help. Within a few short months I will know if Show Inline Help works for inexperienced users. Until then, I am not fighting this battle no more! :D

So, should i enable it in all core components in this PR, or wait inlinehelp review for each component, and enable option when review is complete ?

Leave this PR as is. If it gets RTC and we get official word it's getting merged you can do a new PR for all other core components and then we can then work on PRs for the help strings of each component (one per component) — the latter will need additional review from native English speakers to ensure clarity and conciseness.

@laoneo
Copy link
Member

laoneo commented Mar 2, 2022

I have tested this item ✅ successfully on 80c2f8a


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

@alikon
Copy link
Contributor

alikon commented Mar 2, 2022

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 2, 2022
@cyrezdev
Copy link
Contributor Author

cyrezdev commented Mar 2, 2022

Ideally, we'd need real user testing to know if the assumption that beginners will click that button is true or if we need to revise. Fat chance of that happening.

For another PR: Maybe an alert information message on top of config to inform user of this new functionnality when available ?

Capture d’écran 2022-03-02 à 17 24 58

@bembelimen bembelimen merged commit 3273da7 into joomla:4.1-dev Mar 3, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 3, 2022
@bembelimen
Copy link
Contributor

Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants