Add sidebar component #509
Conversation
sqlpage/templates/shell.handlebars
Outdated
| {{#each (to_array menu_item)}} | ||
| {{#if (or (eq (typeof this) 'object') (and (eq (typeof this) 'string') (starts_with this | ||
| '{')))}} | ||
| {{#with (parse_json this)}} | ||
| {{#if (or (or this.title this.icon) this.image)}} |
There was a problem hiding this comment.
I'm a little afraid of duplicating this logic... Do you think we could use the same markup for the horizontal and the vertical sidebars, with only classes differing between the two ? Or would that be impossible ?
I'm not against changing code style, but we can discuss that separately. Otherwise, PRs are hard to review
Sorry! I was busy. I'll update it asap and review the comment. Thanks! |
|
Hi! I was seeing the changes you have done and I really think... That is all done for now? I don't know if you want to implement more things onto the sidebar |
|
As I said, I'm a little bit hesitant of duplicating the logic of parsing and iterating over the menu items. Is there any chance you can factor it in order to have the same markup for the horizontal and vertical bars, with only classes differing? |
|
I think this is ready :) Can you please resolve the conflicts so I can merge ? |
|
The conflicts probably come from the changes made here to fix the elision of long titles: 6cc4cce#diff-255998f398cb0bb6096065f84876aea7a2ca35ba2c6dda8162e4195e2654ee94 |
|
Okey, I was seeking whats causing merge conflicts, and testing the actual menu-items from the commit you posted, and:
With this new approach, the sidebar menu items get aligned into the right as you can see in the image. I'll check how can I make if it's on sidebar, apply another CSS styles to make the layout more flex |
|
I'm more convinced that the conflicts are now solved. The conflicts showed on the branch are a white-space that no longer exists on the component and the addition of the inline of menu-items. What are your thoughts about this @lovasoa ? |
|
You can merge the main branch of this repo into your branch, then, and the conflicts will disappear here |
|
Cool, it worked. Thanks! |
|
Great, here we are! Thank you! |
|
Argh, I think this broke the menu layout on narrow screens :/ before
after
|
|
Also there is now an underline on the page title on hover
|
|
I'm going to check the CSS. Sorry for the inconvenience |
This behaviour occurs in v.0.26.0. Is not a modification on the sidebar addition :/ |
|
Thank you ! No problem. You can have a look at the mobile layout problem, and I'll take care of the underline, then ! |
|
I found the problem! I'll try to fix it ASAP |
|
I'm updating the https://sql.ophir.dev/examples/layouts.sql page to demonstrate the new sidebar |
I updated the component "Sidebar" started on March.