plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
426 stars 575 forks source link

Back button on personal tools icon in toolbar not displayed. #5938

Closed MostafaMagdyy closed 1 month ago

MostafaMagdyy commented 1 month ago

Describe the bug The back button on the personal tools icon in the toolbar is not displayed.

To Reproduce 1-In the toolbar, click the personal tools icon. 2-Note that the back button is not displayed. 3-Navigate to the volto/src/components/manage/Toolbar directory. 4-Open the PersonalTools.jsx file and observe line 79. 5-Notice that the back button should be displayed.

Expected behavior The back button on the personal tools icon in the toolbar should be displayed. Fixed

Screenshots Wrong

Software ():

MostafaMagdyy commented 1 month ago

Hey, I tried to solve this issue after modifying the CSS for the Toolbar, and it appeared as shown in the screenshot I provided. However, when I tried to test it, I encountered an error regarding the function unloadComponent in the volto/src/components/manage/Toolbar.jsx file. Error2

Previous loaded component i.e(-2) does not have hideToolbarBody property. I tried to solve it by checking if it is undefined to set it to false, but i don't think it is the better way and i do not want to break other behaviours. So, is there any guidance about this error?

stevepiercy commented 1 month ago

Why should a back button be displayed? I don't think this is a bug.

MostafaMagdyy commented 1 month ago

Why should a back button be displayed? I don't think this is a bug.

@stevepiercy It should be displayed as it already exists a button rendered with classname="back "in the PersonalTools.jsx component as i said above. Steve

stevepiercy commented 1 month ago

OK, I see now. It helps to provide a permalink to code, like this:

https://github.com/plone/volto/blob/13d5d60e9923af6fa45383794096f45174c47123/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx#L79

What should happen when clicking the back button, if it were to be displayed? Currently clicking anywhere outside the menu hides it. I assume the same behavior would be desired, and would help with accessibility.

ichim-david commented 1 month ago

@stevepiercy @MostafaMagdyy this is not a bug. The back button shows up when you have a popup opened and you need to go back. That only happens when you click on the "Preferences" option. When you do, you have the right css to show the back button and it works when you click it. personal-preferences

MostafaMagdyy commented 1 month ago

@stevepiercy @MostafaMagdyy this is not a bug. The back button shows up when you have a popup opened and you need to go back. That only happens when you click on the "Preferences" option. When you do, you have the right css to show the back button and it works when you click it. personal-preferences

@ichim-david @stevepiercy this is not the same button I am talking about If you check the PersonalTools.jsx component volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx you will see it is rendered within header component and beside username. Also, the button you are talking about isn't even rendered in the same component, you'll see that the component preferences are pushed in this line volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx Even in the picture I provided, it takes another color, and I only changed its visibility from toolbar.less because it was non-visible. However, as you see. Thank you.

ichim-david commented 1 month ago

@stevepiercy @MostafaMagdyy this is not a bug. The back button shows up when you have a popup opened and you need to go back. That only happens when you click on the "Preferences" option. When you do, you have the right css to show the back button and it works when you click it. personal-preferences

@ichim-david @stevepiercy this is not the same button I am talking about If you check the PersonalTools.jsx component volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx you will see it is rendered within header component and beside username. Also, the button you are talking about isn't even rendered in the same component, you'll see that the component preferences are pushed in this line volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx Even in the picture I provided, it takes another color, and I only changed its visibility from toolbar.less because it was non-visible. However, as you see. Thank you.

Even if it's not the same button because it's from another sub component the functionality of that back button is to go back from a sub component back to the main list and that is probably why that button is hidden and the close doesn't work. There is no point in trying to show it and close it, assume that it is done on purpose.

Also again the advice remains, it's better to add an issue and wait to get some bug confirmation and if it's worth pursuing by a core developer so that you minimize the risk of working in vain and not getting the pull request merged.

We are getting flooded with bug reports that might represent a non-issue to begin with or of low importance and again the chance for a fast turnaround and review will be slim due to limited capacity to review an already big queue of pull requests that are already opened and need to be checked or further worked on or given feedback.

There is a reason we advise new contributors to look at the tickets that have the needs help label as there the contributions are for certain wanted and needed.

ichim-david commented 2 weeks ago

@stevepiercy @MostafaMagdyy this is not a bug. The back button shows up when you have a popup opened and you need to go back. That only happens when you click on the "Preferences" option. When you do, you have the right css to show the back button and it works when you click it. personal-preferences

@ichim-david @stevepiercy this is not the same button I am talking about If you check the PersonalTools.jsx component volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx you will see it is rendered within header component and beside username. Also, the button you are talking about isn't even rendered in the same component, you'll see that the component preferences are pushed in this line volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx Even in the picture I provided, it takes another color, and I only changed its visibility from toolbar.less because it was non-visible. However, as you see. Thank you.

@MostafaMagdyy I was looking today at the functionality of the more section and I found the back button visible from personal tools. That is visible and working as soon as you go 767px and less where you get the mobile menu. Have a look at this video where I show the buttons that are hidden and how they work

https://github.com/plone/volto/assets/152852/924e6960-bdb4-4eec-b153-d274f44c5c0c

And this is the perfect reason why if an issue is added especially by beginners who are not familiar with the codebase it needs to be confirmed first before attempting to fix it as it might not be a bug in the first place.

MostafaMagdyy commented 2 weeks ago

@stevepiercy @MostafaMagdyy this is not a bug. The back button shows up when you have a popup opened and you need to go back. That only happens when you click on the "Preferences" option. When you do, you have the right css to show the back button and it works when you click it. personal-preferences

@ichim-david @stevepiercy this is not the same button I am talking about If you check the PersonalTools.jsx component volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx you will see it is rendered within header component and beside username. Also, the button you are talking about isn't even rendered in the same component, you'll see that the component preferences are pushed in this line volto/packages/volto/src/components/manage/Toolbar/PersonalTools.jsx Even in the picture I provided, it takes another color, and I only changed its visibility from toolbar.less because it was non-visible. However, as you see. Thank you.

@MostafaMagdyy I was looking today at the functionality of the more section and I found the back button visible from personal tools. That is visible and working as soon as you go 767px and less where you get the mobile menu. Have a look at this video where I show the buttons that are hidden and how they work

https://github.com/plone/volto/assets/152852/924e6960-bdb4-4eec-b153-d274f44c5c0c

And this is the perfect reason why if an issue is added especially by beginners who are not familiar with the codebase it needs to be confirmed first before attempting to fix it as it might not be a bug in the first place.

This is great @ichim-david i appreciate your help, I do not catch that was for mobile menu in the first place. Thanks a lot for this clarification.