vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: settings background script setup #2104

Merged
konrad merged 1 commits from dpschen/frontend:feature/feat-settings-background-script-setup into main 2022-09-01 16:09:52 +00:00
Conversation 13 Commits 1 Files Changed 8 +275 -244
dpschen commented 2022-07-07 18:24:59 +00:00
Member
Copy Link

This is already a bit older. I did the conversion without testing. So probably there are some bugs.

This is already a bit older. I did the conversion without testing. So probably there are some bugs.
frederick commented 2022-07-07 18:29:42 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://2104-feature-feat-settings-background--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://2104-feature-feat-settings-background--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 commented 2022-07-07 20:40:47 +00:00
Owner
Copy Link

Can you investigate the CI failure? It seems unrelated to the changes you made in the PR but it only happens here.

Can you investigate the CI failure? It seems unrelated to the changes you made in the PR but it only happens here.
konrad commented 2022-07-08 15:23:30 +00:00
Owner
Copy Link

Looks like this was caused by recent changes in the api, fixed in a073cfac66 - can you rebase on main?

Looks like this was caused by recent changes in the api, fixed in https://kolaente.dev/vikunja/frontend/commit/a073cfac66b7b82b1b137f02385402f9c2d6cfdd - can you rebase on main?
dpschen force-pushed feature/feat-settings-background-script-setup from 1edd1f0a92 to 4c1422e635 2022-07-08 19:55:24 +00:00 Compare
konrad commented 2022-07-08 21:19:14 +00:00
Owner
Copy Link

Looks like the styling for unsplash background seelction is now broken:

image

And there's a "create" button on the bottom right corner:

image

(not sure if that hasn't been there before)

Other than that it seems to work fine.

Looks like the styling for unsplash background seelction is now broken: ![image](/attachments/c1ee822f-d2aa-4933-9c6f-939f3e4193f7) And there's a "create" button on the bottom right corner: ![image](/attachments/7bcdb22e-360a-4186-b78e-df6b5e9498e5) (not sure if that hasn't been there before) Other than that it seems to work fine.
image.png
321 KiB
image.png
321 KiB
image.png
44 KiB
image.png
konrad commented 2022-07-08 21:20:21 +00:00
Owner
Copy Link

Okay looks like these issues already exist on the main branch, interesting.

Would you mind taking a look into fixing these? I think it makes sense to merge this PR first or base the changes upon this one.

Okay looks like these issues already exist on the main branch, interesting. Would you mind taking a look into fixing these? I think it makes sense to merge this PR first or base the changes upon this one.
dpschen commented 2022-07-09 17:26:39 +00:00
Author
Member
Copy Link

Okay looks like these issues already exist on the main branch, interesting.

Would you mind taking a look into fixing these? I think it makes sense to merge this PR first or base the changes upon this one.

Will investigate! 👍

> Okay looks like these issues already exist on the main branch, interesting. > Would you mind taking a look into fixing these? I think it makes sense to merge this PR first or base the changes upon this one. Will investigate! 👍
dpschen force-pushed feature/feat-settings-background-script-setup from 4c1422e635 to e17faaa7b2 2022-07-09 19:01:58 +00:00 Compare
dpschen commented 2022-07-09 19:04:31 +00:00
Author
Member
Copy Link

The general issues with styling in the modal should be solved by #2109.

This branch now is based on that fix.

I'm not sure about how the buttons should be though.
Can you explain what is wrong here / should be changed or fixed?

The general issues with styling in the modal should be solved by #2109. This branch now is based on that fix. I'm not sure about how the buttons should be though. Can you explain what is wrong here / should be changed or fixed?
dpschen changed title from WIP: feat: settings background script setup to feat: settings background script setup 2022-07-10 11:33:01 +00:00
dpschen requested review from konrad 2022-07-10 11:33:07 +00:00
konrad was assigned by dpschen 2022-07-10 11:33:13 +00:00
dpschen added the
kind/feature
area/internal-code
 labels 2022-07-10 11:33:40 +00:00
konrad reviewed 2022-07-11 10:16:02 +00:00
src/views/list/settings/background.vue Outdated
@ -20,3 +20,3 @@
<x-button
:loading="backgroundUploadService.loading"
@click="$refs.backgroundUploadInput.click()"
@click="backgroundUploadInput?.click()"
konrad commented 2022-07-11 10:14:52 +00:00
Owner

Does this generate an error message when backgroundUploadInput is undefined or null? Or is vue smart enough to figure this out and prevent an error?

Does this generate an error message when `backgroundUploadInput` is undefined or null? Or is vue smart enough to figure this out and prevent an error?
dpschen commented 2022-07-21 23:39:09 +00:00
Author
Member

I think this compiles to @click="null"

I think this compiles to `@click="null"`
✅ 1
dpschen marked this conversation as resolved
konrad commented 2022-07-11 10:17:08 +00:00
Owner
Copy Link

The general issues with styling in the modal should be solved by #2109.

Thanks!

I'm not sure about how the buttons should be though.
Can you explain what is wrong here / should be changed or fixed?

Mostly it's the create button which does not make sense here IMHO. I think we can get rid of it completely.

> The general issues with styling in the modal should be solved by #2109. Thanks! > I'm not sure about how the buttons should be though. Can you explain what is wrong here / should be changed or fixed? Mostly it's the create button which does not make sense here IMHO. I think we can get rid of it completely.
dpschen commented 2022-08-04 21:09:45 +00:00
Author
Member
Copy Link

Mostly it's the create button which does not make sense here IMHO. I think we can get rid of it completely.

@konrad
The button is provied by the CreateEdit component.
How would you adjust that in order to optionally show that button?

> Mostly it's the create button which does not make sense here IMHO. I think we can get rid of it completely. @konrad The button is provied by the CreateEdit component. How would you adjust that in order to optionally show that button?
konrad commented 2022-08-04 21:37:44 +00:00
Owner
Copy Link

@dpschen Even though a bit ugly the best way is probably to add another prop to the component.

@dpschen Even though a bit ugly the best way is probably to add another prop to the component.
dpschen commented 2022-08-05 17:05:35 +00:00
Author
Member
Copy Link

@dpschen Even though a bit ugly the best way is probably to add another prop to the component.

I always thought that you could check the attrs if onPrimary is set. I tested that and it was never set. 🤷‍♂️

> @dpschen Even though a bit ugly the best way is probably to add another prop to the component. I always thought that you could check the attrs if `onPrimary` is set. I tested that and it was never set. 🤷‍♂️
konrad commented 2022-08-08 11:40:38 +00:00
Owner
Copy Link

Isn't onPrimary an event handler? (didn't look at the code, this is from memory)

I thought of adding another property showPrimaryActionButton even though that's a bit ugly.

Isn't `onPrimary` an event handler? (didn't look at the code, this is from memory) I thought of adding another property `showPrimaryActionButton` even though that's a bit ugly.
dpschen commented 2022-08-16 07:28:28 +00:00
Author
Member
Copy Link

Isn't onPrimary an event handler? (didn't look at the code, this is from memory)

I thought of adding another property showPrimaryActionButton even though that's a bit ugly.

It is. But based on this explanation the attrs object should contain onPrimary.

> Isn't `onPrimary` an event handler? (didn't look at the code, this is from memory) > > I thought of adding another property `showPrimaryActionButton` even though that's a bit ugly. It is. But based on [this explanation](https://v3-migration.vuejs.org/breaking-changes/listeners-removed.html#_3-x-syntax) the attrs object should contain `onPrimary`.
dpschen force-pushed feature/feat-settings-background-script-setup from e17faaa7b2 to 5e3eece3cc 2022-08-18 16:16:40 +00:00 Compare
dpschen force-pushed feature/feat-settings-background-script-setup from 5e3eece3cc to 84168a98e4 2022-08-18 16:27:55 +00:00 Compare
dpschen force-pushed feature/feat-settings-background-script-setup from 84168a98e4 to 0dcb433196 2022-08-18 17:01:22 +00:00 Compare
konrad approved these changes 2022-09-01 16:09:39 +00:00
konrad left a comment
Owner

Thanks!

Thanks!
konrad merged commit ff655808b3 into main 2022-09-01 16:09:52 +00:00
konrad deleted branch feature/feat-settings-background-script-setup 2022-09-01 16:09:52 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
konrad
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
konrad
3 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#2104
No description provided.
Delete Branch "dpschen/frontend:feature/feat-settings-background-script-setup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?