-
Notifications
You must be signed in to change notification settings - Fork 88
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
Improvements to AppSidebar component #1090
Comments
cc @jancborchardt |
|
Small point about this by the way: Yes the title should be rendered as a h2 as you say, and in addition it should also be rendered as h2 when in edit mode, just that an input border happens to be around it. No movement of the text from editing to actual display mode would be ideal, as that makes it look rock solid. |
@nextcloud/vuejs Would it be ok to use JSX to write render functions? JSX seems to be a lot easier to write and read than plain javascript in that case, see https://vuejs.org/v2/guide/render-function.html#JSX The drawback is that we would need a babel plugin, but since NPM takes care of installing it, it should be ok I guess. |
You don't need jsx to write render functions :) |
@skjnldsv I know, I wrote these render functions 😉 return createElement(
'anchored-heading', {
props: {
level: 1
}
}, [
createElement('span', 'Hello'),
' world!'
]
) to the JSX version return (
<AnchoredHeading level={1}>
<span>Hello</span> world!
</AnchoredHeading>
) (taken from the vue.js docs: https://vuejs.org/v2/guide/render-function.html#JSX) And it gets even clearer when you have props, callbacks, attributes, etc. |
Oups 🙈🙈 I'm fine with jsx to be honest. It's not like the component is crazy simple already. Maybe we can handle the slot render separately from the AppNavigation so we'll still keep the Vue template for the simple part? I'll let your judge the overall complexity. I'm sure there is a good compromise between all of this! 🤗😉 |
Shall we allow to drag the left border of the sidebar to allow adjusting its width (in a range of a min-width and max-width)? We got such a request for the Tasks app nextcloud/tasks#1185. I think it could be nice to give the sidebar content a bit more space if needed. |
Closing, as all proposed features are implemented. |
I currently move the right details view of the Tasks app to the AppSidebar component of nextcloud-vue, see nextcloud/tasks#1030. I noticed a few things with
AppSidebar
which would limit the functionality of the Tasks app. This issue is to keep track of what needs to be improved on theAppSidebar
component to make it usable for Tasks. I will likely add more items to the list, since I am far from done with nextcloud/tasks#1030.Allow to specify a custom icon for the starred buttonWe will not show the star icon for the priority in the header anymore. The completed checkbox is colored in red, yellow or blue to show the priority in the header, priority can be set in the details.The Tasks app has multiple levels of priority for a task, low, medium and high. Each level is indicated by a different star color. Hence, the star icon needs to be adjustable (color would be enough, but I guess it is easier to just provide an icon).
Make the star icon biggerThis is a matter of taste, but I think it is a bit to small (see image above for a comparison of nextcloud-vue and current Tasks star size).
The title should be rendered as
h2
and the input field should be visible after clicking the title. This can be optional, but it is a requirement for the next point. Also, I think it looks weird to permanently show the input field, since the title is then quite small and looks very different to the non-editable title.Compare non-editable
and editable
In Tasks we linkify links in the task summary, so you can navigate there without copy/paste. The same should be (optionally) done for the title in
AppSidebar
. This also means the editable title cannot be shown in an input field permanently.The text was updated successfully, but these errors were encountered: