ToU: navigation panel component#1140
Conversation
|
Deployment previews on netlify for branch
|
There was a problem hiding this comment.
I think you may have accidentally added and commited this whole file
There was a problem hiding this comment.
Ah well for now I just copied a version from #1130 to have some content as a placeholder
Currently in this file, in this PR, the interesting bits are basically only around Line 5-7 where the new component gets used
<v-col cols="11" md="4" order-md="last">
<TermsOfUseNavigationPanel />
</v-col>
Maybe after we are sure the approach is the right one I can clean it up so it only provides the new components, then it can get integrated in the other PR
There was a problem hiding this comment.
right! sorry. You even mentioned that in the PR description
|
|
||
| </script> | ||
|
|
||
| <style scoped></style> |
There was a problem hiding this comment.
good idea to leave this there (clearly marked as scoped) as a service to the future editors of this component
There was a problem hiding this comment.
I guess this was added as an attempt to remove the list from the NavigationPanel but not currently used?
| }, | ||
| methods: { | ||
| isCurrentPage (routeName) { | ||
| return this.$route.name === routeName |
There was a problem hiding this comment.
I think I'd rather we also pass in the current link as a prop to this component rather than have it magically determined it by the route name.
While we just have two hardcoded routes this works but once we are getting the policy list over the wire I don't think we'll be able to rely on the route name. We'll need to look at the params of the route and I think at that point it's probably cleaner for the caller of this component to specify which is the current page.
To try and give an example I think we will need to handle:
- current
- upcoming
- 2029-01-01
- 2022-01-01
| @@ -0,0 +1,41 @@ | |||
| <template> | |||
| <v-expansion-panels v-bind:value="$vuetify.breakpoint.mdAndUp ? 0 : null" v-if="links"> | |||
There was a problem hiding this comment.
Very nice clean vuetify2 way to handle the responsiveness. I love it.
I did have to go hunting to confirm value is the name for prop that controls if the panel is open or closed but that's not on you; for me that's not a super clear prop name :D
3 participants
WIP - current changes on routes and ToU pages themselves are only to get the right structure that gets currently build in #1130
once that PR is approved we can merge/rebase it into this one
that means interesting changes in here are currently only regarding the navigation panel
I went for a generic component, wrapped in a "Terms of Use" specific one, which can just be included on any of the ToU pages.
Another approach could be to store the link array in a plain js module and pass it on as a prop to the generic navigation panel component, but this current approach felt more "vue" to me. I am very interested to hear what the team prefers; there are probably more sensible ways i did not think of.
https://phabricator.wikimedia.org/T407632