-
-
Notifications
You must be signed in to change notification settings - Fork 215
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
Improve Sidebar Menu's button and label placement #9481
Comments
Hi @latin-panda I would like to contribute to this issue if I can. Thank you |
Thanks @GallyTi. We need a quick confirmation from our designer @n-orlowski to ensure the proposed solution is okay. @n-orlowski, are these solutions explained okay? Checking because I think you had some thoughts on the second point about long names. |
Hi @latin-panda. I'm very sorry to tell you this, but I was unable to run this project locally on my computer. Neither from WSL 2, nor from a VM of Ubuntu 22.04. I can't install the dependencies. I have been trying to run this project for about 5 hours. So I apologize, but unfortunately I can't participate in this project. The main problem was with installing dependencies via: npm ci --legacy-peer-deps when running npm run build-dev I get an error message:
build-prepare: cleaning build directory Error: Cannot find module '/home/gally/cht-core/node_modules/request-promise-native/lib/r Even when i was trying to install these dependencies i was not able to run this project localy. |
@GallyTi, sorry to hear about the problems, I appreciate your efforts in setting up the environment.
|
EDIT 1: So after reinstalling it is working properly EDIT 2:
Error: npm exited with 1 Node.js v20.18.0 Old message: So after updating a node.js to version 20.18.0 So then i run npm ci --legacy-peer-deps in ~/cht-core which resulted to install some deppendencies BUT: As you can see the npm ci gives me this: And when i tried to run a npm run build-dev I do get this: So any recommendation? Thank you 😄 |
@GallyTi Thanks for trying it out again, I'm glad it's working now. > npm error npm warn old lockfile The package-lock.json file was created with an old version of npm,
> npm error npm warn old lockfile so supplemental metadata must be fetched from the registry. CHT has its own package-lock.js, which is very important because it ensures the installation of dependencies with the correct version by using |
So, i tried to install it as you described, and fully clean install, where i will attach 2 txt files with full terminal. Both of them are unsuccessful. So solution for windows is not working, i mean on my machine. Without opening these files you can see same problem for nstalling-cht-core-on-ubuntu-windows-log-from-terminal.txt ass you mentioned. installing-cht-core-without-package-lock-json.txt Btw, i can work on these changes in this issue, so i will wait for response from your designer. Thanks, for your support and sorry for troubles... @latin-panda :) |
Hi both, and thanks @GallyTi for picking this up! For the first improvement my recommendation would be to match the alignment of the "X" to the hamburger icon and subsequently left align the icons below it when the drawer is open (instead of making the clickable area larger) For the second improvement, best practice is to keep navigation levels to one line but as we are designing for flexibility I'm ok with the proposed solution (two lines plus elipses), however we should encourage projects to use shorter label text where possible |
Okay, you can assignee this to me. I will try to improve this, and as soon as i will have a solution for this i will provide it by screenshots. |
Thanks @GallyTi for picking this up! You can find documentation that can help you get started . If you have any questions, let us know! |
Hi @latin-panda and @n-orlowski. So i made these changes, as you requested: For the first solution, I have made as close as unopened state, so the Close button is same width and same clickable size as burger menu. Also i have made the adjustments for the menu items so they align properly with close button. Same changes for mobile menu, where it is aligned to burger menu: If you have anything to change tell me, i will make a pull request. |
Thanks @GallyTi ! The double lines in the side nav looks great 🎉 The menu title and text in the drawer is a little far from their corresponding icons, can we keep the original spacing? 🙏 |
Hi, @n-orlowski. So I've made the changes you requested. For now it look like this: Is it good, or? Thank you :) I have achieved that by using -15px margin left for text, so we can have the icon at the same position. |
Looks good to me! Thanks for your help @GallyTi 💪 |
@GallyTi I'll be reviewing your PR tomorrow 🤩 |
Describe the issue
During exploration testing we got 2 feedback for improving the Sidebar Menu.
Describe the improvement you'd like
1.. It would be nice to be able to close the hamburger menu from the same position without having to move the cursor to the left.
1.mp4
2.. When I chose Bamanankan (Bambara) , on Desktop, the word for messages is too long, and it needs to be cut. I think it is great that nav bar still looks the same, but I was wondering if it ok to cut the work/phrase?
Mobile is okay:
The text was updated successfully, but these errors were encountered: