feat: simplify namespace search #1835

Merged
konrad merged 1 commits from dpschen/frontend:feature/feat-simplify-namespace-selection into main 2022-04-25 17:38:58 +00:00
Member
No description provided.
Member

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1835-feature-feat-simplify-namespace---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://1835-feature-feat-simplify-namespace---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.
konrad requested changes 2022-04-18 17:01:13 +00:00
konrad left a comment
Owner

It seems like the search does not work - the multiselect is missing completely:

image

It seems like the search does not work - the multiselect is missing completely: ![image](/attachments/c3c66584-852e-4f7e-996c-076b78e9b2d2)
konrad reviewed 2022-04-18 17:01:40 +00:00
@ -10,3 +10,3 @@
{{ $t('list.duplicate.text') }}
</p>
<namespace-search @selected="selectNamespace"/>
<Multiselect

Looks like you did not import the multiselect component.

Looks like you did not import the multiselect component.
Author
Member

Done

Done
dpschen marked this conversation as resolved
Owner

Would you mind adding a Cypress test for list duplication while we're at it?

Would you mind adding a Cypress test for list duplication while we're at it?
dpschen force-pushed feature/feat-simplify-namespace-selection from 1b09259239 to 1f1abf083e 2022-04-18 19:17:23 +00:00 Compare
Author
Member

Can we see this branch as a bit more complex than necessary conversion to script setup?

Because I don't think it's a good idea to introduce tests to files where I would change much more if I would really rewrite instead of only convert them.
In this case e.g. aggregating the not component related logic somewhere.

Can we see this branch as a bit more complex than necessary conversion to script setup? Because I don't think it's a good idea to introduce tests to files where I would change much more if I would really rewrite instead of only convert them. In this case e.g. aggregating the not component related logic somewhere.
Owner

Sure. I just figured if we had a test for this feature, the missing multiselect import could have been caught earlier 🙃

Sure. I just figured if we had a test for this feature, the missing multiselect import could have been caught earlier 🙃
Owner

Now duplicating a list fails with this error:

image

Now duplicating a list fails with this error: ![image](/attachments/5d37cbe2-7635-4882-8c74-279f23324e92)
4.0 KiB
dpschen force-pushed feature/feat-simplify-namespace-selection from 1f1abf083e to 67c6f1ddca 2022-04-18 20:17:39 +00:00 Compare
Author
Member

The missing import would probably be catched by the improved vue linting.
But I get what you mean ;)

The missing import would probably be catched by the improved vue linting. But I get what you mean ;)
Author
Member

@konrad:

This is going in the direction I would like to create tests in the future:
#1800 (comment)

Would that be fine for you? And if yes can we merge this pull request?

@konrad: This is going in the direction I would like to create tests in the future: https://kolaente.dev/vikunja/frontend/pulls/1800#issuecomment-29685 Would that be fine for you? And if yes can we merge this pull request?
dpschen requested review from konrad 2022-04-23 11:51:47 +00:00
konrad was assigned by dpschen 2022-04-23 11:52:05 +00:00
konrad approved these changes 2022-04-25 17:38:47 +00:00
konrad left a comment
Owner

Looks great!

Looks great!
konrad merged commit 8578225982 into main 2022-04-25 17:38:58 +00:00
konrad deleted branch feature/feat-simplify-namespace-selection 2022-04-25 17:38:58 +00:00
This repo is archived. You cannot comment on pull requests.
No description provided.