[NF] Menu item visibility indicator #11998

Merged
merged 7 commits into from Nov 4, 2016

Conversation

Projects
None yet
8 participants
@Devportobello
Contributor

Devportobello commented Sep 9, 2016

Context

The new feature of the menu component allowing to show/hide a menu item from params, when we watch all items we can't know if item is hidden or not.

Testing Instructions

  • Create a menu with some items, hide some with the param: "Display in Menu"
  • Take a look at the list of items, unable to know who is hidden or not

Patch

  • Now you can know hidden or not

Work in progress

i dunno if the icon is at the right place (before title)

Devportobello added some commits Sep 9, 2016

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Sep 9, 2016

Contributor

I see what you are trying to do but I dont think this is the correct approach.

As the majority of menu items will be visible I dont see the need to always show the eye-icon

So I would probably only show the icon for items that are invisible and I would display that after the title.

Also you need some sort of tooltip/popover to explain the meaning of the icon

Contributor

brianteeman commented Sep 9, 2016

I see what you are trying to do but I dont think this is the correct approach.

As the majority of menu items will be visible I dont see the need to always show the eye-icon

So I would probably only show the icon for items that are invisible and I would display that after the title.

Also you need some sort of tooltip/popover to explain the meaning of the icon

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Sep 9, 2016

Contributor

Instead of showing an icon (with a tooltip) maybe you can just show a small label that says "hidden" behind the title. Similar to how we show unpublished articles in frontend to the admins.

Contributor

Bakual commented Sep 9, 2016

Instead of showing an icon (with a tooltip) maybe you can just show a small label that says "hidden" behind the title. Similar to how we show unpublished articles in frontend to the admins.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Sep 9, 2016

Contributor

That would work too

Contributor

brianteeman commented Sep 9, 2016

That would work too

@Devportobello

This comment has been minimized.

Show comment
Hide comment
@Devportobello

Devportobello Sep 12, 2016

Contributor

@brianteeman @Bakual
Ye i agree with you
Label will add a new text translation, adding "JHIDDEN" in "en-GB.ini" is the right approach ?

Contributor

Devportobello commented Sep 12, 2016

@brianteeman @Bakual
Ye i agree with you
Label will add a new text translation, adding "JHIDDEN" in "en-GB.ini" is the right approach ?

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Sep 12, 2016

Contributor

I wouldn't make it a global string. It's only used in the menu manager. Just put it into the en-GB.com_menus.ini and name it something like COM_MENUS_LABEL_HIDDEN.

Contributor

Bakual commented Sep 12, 2016

I wouldn't make it a global string. It's only used in the menu manager. Just put it into the en-GB.com_menus.ini and name it something like COM_MENUS_LABEL_HIDDEN.

ux
@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Sep 12, 2016

Contributor

I have tested this item successfully on 0b5251d


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

Contributor

brianteeman commented Sep 12, 2016

I have tested this item successfully on 0b5251d


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

Devportobello added some commits Sep 12, 2016

cs
@gwsdesk

This comment has been minimized.

Show comment
Hide comment
@gwsdesk

gwsdesk Sep 18, 2016

I have tested this item successfully on ba68b2d


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

gwsdesk commented Sep 18, 2016

I have tested this item successfully on ba68b2d


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

@conconnl

This comment has been minimized.

Show comment
Hide comment
@conconnl

conconnl Nov 4, 2016

I have tested this item successfully on ba68b2d

Tested successfully at PBF NL


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

conconnl commented Nov 4, 2016

I have tested this item successfully on ba68b2d

Tested successfully at PBF NL


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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Nov 4, 2016

Contributor

I have tested this item successfully on ba68b2d

After applying the patch the hidden notice shows behind the menu item.


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

Contributor

roland-d commented Nov 4, 2016

I have tested this item successfully on ba68b2d

After applying the patch the hidden notice shows behind the menu item.


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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Nov 4, 2016

Contributor

Setting to RTC


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

Contributor

roland-d commented Nov 4, 2016

Setting to RTC


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

@joomla-cms-bot joomla-cms-bot added RTC and removed Language Change labels Nov 4, 2016

@brianteeman brianteeman added this to the Joomla 3.7.0 milestone Nov 4, 2016

@roland-d roland-d merged commit adb8532 into joomla:staging Nov 4, 2016

3 checks passed

JTracker/HumanTestResults Human Test Results: 3 Successful 0 Failed.
Details
continuous-integration/drone the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot added Language Change and removed RTC labels Nov 4, 2016

@Devportobello Devportobello deleted the Devportobello:menuitemVisibilityIndicator branch Nov 4, 2016

nvyush added a commit to nvyush/joomla-cms that referenced this pull request Nov 9, 2016

[NF] Menu item visibility indicator (#11998)
* Menuitem visibility indicator

* cs test1

* cs test2

* ux

* moveLabelAfterAlias

* handleInvalidJson

* cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment