WIP: feat: route modals everywhere #2735

Draft
dpschen wants to merge 15 commits from dpschen/frontend:feature/route-modals-everywhere into main
Collaborator

This enables route modals everywhere, where possible.

This is a major usability improvement, because you can go back to the route where you came from much faster.

This enables route modals everywhere, where possible. This is a major usability improvement, because you can go back to the route where you came from much faster.
dpschen added the kind/feature label 2 months ago
dpschen added 1 commit 2 months ago
continuous-integration/drone/pr Build is failing Details
eb6d239213
feat: route modals everywhere
dpschen reviewed 2 months ago
<!-- https://github.com/vuejs/rfcs/pull/449 -->
Poster
Collaborator

This component makes it possible to optionally wrap around another component. If the is prop defines a component it will be rendered as a wrapper around the slot content. If the is prop is undefined there won't be any wrapper.

This component makes it possible to optionally wrap around another component. If the `is` prop defines a component it will be rendered as a wrapper around the slot content. If the `is` prop is undefined there won't be any wrapper.
dpschen marked this conversation as resolved
defineProps<{ is: any }>()
const attrs = useAttrs()
console.log(JSON.parse(JSON.stringify(attrs)))
Poster
Collaborator

Remove this

Remove this
dpschen marked this conversation as resolved
<modal
:enabled="Boolean(currentModal)"
<component
Poster
Collaborator

Every modal component now has to provide it's own modal.

This makes it possible for us to have different modal implementations. Currently we do this by changing the type prop of the modal. It might be easier for us if we create something like a BaseModal to abstract the general modal functionality and then use that to create styled Modals with specific functionality. For example we coudl create a dedicated Dialog modal (this might actually be the same as the current create-edit, I'm not sure here if that was the intended use @konrad).

Every modal component now has to provide it's own modal. This makes it possible for us to have different modal implementations. Currently we do this by changing the `type` prop of the modal. It might be easier for us if we create something like a `BaseModal` to abstract the general modal functionality and then use that to create styled Modals with specific functionality. For example we coudl create a dedicated `Dialog` modal (this might actually be the same as the current `create-edit`, I'm not sure here if that was the intended use @konrad).
Owner

That sounds like it could be a good idea.

IIRC my main goal with the create-edit component was to be able to easily re-use a shell for creating or editing.

That sounds like it could be a good idea. IIRC my main goal with the `create-edit` component was to be able to easily re-use a shell for creating or editing.
<template>
<modal @close="$router.back()" :overflow="true" :wide="wide">
<modal :overflow="true" :wide="wide" #default="{ onClose }">
Poster
Collaborator

I removed the @close event definitions on all internal modals because what happens when that event get's fired should instead be defined by the parent. By removing this the parents onClose / @close should automatically be passed to the <modal>.
By passing the onClose then to the slot content, the slot is still able to use the method.

I removed the `@close` event definitions on all internal modals because what happens when that event get's fired should instead be defined by the parent. By removing this the parents `onClose` / `@close` should automatically be passed to the `<modal>`. By passing the `onClose` then to the slot content, the slot is still able to use the method.
function primary() {
emit('create')
emit('primary')
function primary(onClose: () => void) {
Poster
Collaborator

We pass onClose as a callback parameter to the primary emit so that it can reuse the onClose after the primary action finished (unsure if that makes sense).

We pass `onClose` as a callback parameter to the `primary` emit so that it can reuse the `onClose` after the primary action finished (unsure if that makes sense).
<Teleport to="body">
<!-- FIXME: transition should not be included in the modal -->
<CustomTransition :name="transitionName" appear>
<CustomTransition :name="transitionName" appear :appear-class="transitionName">
Poster
Collaborator

The appear transition is currently broken. Fix this.

The appear transition is currently broken. Fix this.
scrollLock.value = props.enabled
})
function onClose() {
Poster
Collaborator

FIXME: either we use an implicit attrs.onClose or we explicit define a close emit. Both should not be mixed.

FIXME: either we use an implicit `attrs.onClose` or we explicit define a `close` emit. Both should not be mixed.
const historyState = computed(() => route.fullPath && window.history.state)
if (historyState.value) {
if (historyState.value === undefined) {
Poster
Collaborator

Something was broken here: since we checked for historyState.value in the if condition it could never have a value in the else cause.

Something was broken here: since we checked for `historyState.value` in the `if` condition it could never have a value in the `else` cause.
variant="tertiary"
class="is-danger"
@click.prevent.stop="removeBackground"
@click="removeBackground(onClose)"
Poster
Collaborator

We should not need to use .prevent and .stop with the vueuse onClickOutside.

We should not need to use `.prevent` and `.stop` with the vueuse `onClickOutside`.
<template>
<OptionalWrapper :is="isModal && Modal" variant="scrolling">
Poster
Collaborator

By using the OptionalWrapper we can use components that can be used with or without Modal

By using the OptionalWrapper we can use components that can be used with or without Modal
defineEmits(['close'])
const attrs = useAttrs()
console.log(JSON.parse(JSON.stringify(attrs)))
Poster
Collaborator

Remove log

Remove log
dpschen self-assigned this 2 months ago
dpschen force-pushed feature/route-modals-everywhere from eb6d239213 to 130620b077 2 months ago
Collaborator

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://2735-feature-route-modals-everywhere--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://2735-feature-route-modals-everywhere--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
dpschen force-pushed feature/route-modals-everywhere from 130620b077 to a6765638c1 2 months ago
dpschen added 11 commits 2 months ago
dpschen added 4 commits 2 months ago
dpschen force-pushed feature/route-modals-everywhere from 87f8e66e34 to cd72db7287 2 months ago
dpschen force-pushed feature/route-modals-everywhere from cd72db7287 to de7d004c6e 2 months ago
Some checks failed
continuous-integration/drone/pr Build is failing
Required
Details
This pull request has changes conflicting with the target branch.
package.json
pnpm-lock.yaml
src/components/tasks/partials/singleTaskInList.vue
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#2735
Loading…
There is no content yet.