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

feat: move update from navigation to app #2997

Merged
dpschen merged 3 commits from dpschen/frontend:feature/move-update-to-app into main 2023-02-03 16:34:49 +00:00
Conversation 8 Commits 3 Files Changed 7 +56 -62
dpschen commented 2023-01-23 21:24:04 +00:00
Member
Copy Link
No description provided.
dpschen added the
area/internal-code
 label 2023-01-23 21:24:04 +00:00
konrad was assigned by dpschen 2023-01-23 21:24:04 +00:00
frederick commented 2023-01-23 21:33:14 +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://2997-feature-move-update-to-app--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://2997-feature-move-update-to-app--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 2023-01-24 21:59:17 +00:00
Owner
Copy Link

Any way to test this?

Any way to test this?
dpschen commented 2023-01-24 22:03:25 +00:00
Author
Member
Copy Link

I guess you refer mostly to the changes in Update.vue, right?
You can build a version then serve it locally via https-localhost.

Open the app and let the service worker load the stuff.

Rebuild the app with a mini change and serve it again. The update should appear.

I guess you refer mostly to the changes in `Update.vue`, right? You can build a version then serve it locally via [https-localhost](https://github.com/daquinoaldo/https-localhost). Open the app and let the service worker load the stuff. Rebuild the app with a mini change and serve it again. The update should appear.
👍 1
konrad requested changes 2023-01-25 14:13:45 +00:00
konrad left a comment
Owner

Looks like the update notice is a) only visible when not logged in and b) even there not fully:

image

Looks like the update notice is a) only visible when not logged in and b) even there not fully: ![image](/attachments/6ca1b6ab-1303-4201-9226-b6fcf7967acf)
image.png
97 KiB
konrad commented 2023-01-26 12:10:42 +00:00
Owner
Copy Link

Fixed the z-index on the banner so that now it stays on top at all times.

Fixed the z-index on the banner so that now it stays on top at all times.
dpschen was assigned by konrad 2023-01-26 12:11:13 +00:00
dpschen commented 2023-01-26 13:03:02 +00:00
Author
Member
Copy Link

I'm confused that the the dark version works without using :global() since the style is scoped and the dark class is neither set from inside nor on the element itself.

I'm confused that the the dark version works without using `:global()` since the style is scoped and the `dark` class is neither set from inside nor on the element itself.
konrad commented 2023-01-26 14:28:00 +00:00
Owner
Copy Link

I'm confused that the the dark version works without using :global() since the style is scoped and the dark class is neither set from inside nor on the element itself.

It actually does not work at all with the :global() helper. The generated class looks like this without it: .dark .update-notification[data-v-4c295d9f] - the scoped data-attribute is only applied to the .update-notification class. With the :global() helper it compiles to .dark only.

This might be a bug in the helper but I'd say we can leave it like this for now.

> I'm confused that the the dark version works without using `:global()` since the style is scoped and the `dark` class is neither set from inside nor on the element itself. It actually does not work at all with the `:global()` helper. The generated class looks like this without it: `.dark .update-notification[data-v-4c295d9f]` - the scoped data-attribute is only applied to the `.update-notification` class. With the `:global()` helper it compiles to `.dark` only. This might be a bug in the helper but I'd say we can leave it like this for now.
konrad approved these changes 2023-01-26 14:49:38 +00:00
dpschen commented 2023-01-26 19:57:44 +00:00
Author
Member
Copy Link

Seems like a bug: https://vuejs.org/api/sfc-css-features.html#global-selectors

Seems like a bug: https://vuejs.org/api/sfc-css-features.html#global-selectors
dpschen reviewed 2023-01-27 15:01:20 +00:00
src/App.vue Outdated
@ -1,5 +1,6 @@
<template>
<ready>
<Update/>
dpschen commented 2023-01-26 12:54:21 +00:00
Author
Member

If we move <Update> below <Notification> can we then remove the z-index?.

We might want to move <KeyboardShortcuts> above <Notification> at the same time. I'm unsure but couldn't the current order make the Shortcuts appear above the Notifications?

If we move `<Update>` below `<Notification>` can we then remove the z-index?. We might want to move `<KeyboardShortcuts>` above `<Notification>` at the same time. I'm unsure but couldn't the current order make the Shortcuts appear above the Notifications?
konrad commented 2023-01-29 14:21:16 +00:00
Owner

That seems to work on the login page but does not in the rest of Vikunja. Let's leave it with the z-index.

I'm unsure but couldn't the current order make the Shortcuts appear above the Notifications?

Not sure, I checked it and it seems to work fine like it is now.

That seems to work on the login page but does not in the rest of Vikunja. Let's leave it with the z-index. > I'm unsure but couldn't the current order make the Shortcuts appear above the Notifications? Not sure, I checked it and it seems to work fine like it is now.
dpschen force-pushed feature/move-update-to-app from 28fad65d9d to 058cfe1318 2023-02-03 16:05:17 +00:00 Compare
dpschen force-pushed feature/move-update-to-app from 058cfe1318 to eaeddda4e4 2023-02-03 16:25:50 +00:00 Compare
dpschen scheduled this pull request to auto merge when all checks succeed 2023-02-03 16:26:38 +00:00
dpschen commented 2023-02-03 16:27:40 +00:00
Author
Member
Copy Link

I found out why the dark styles were broken. I think I found out why we had those bulma-css-variable problems…

But not in this pull request.

I found out why the dark styles were broken. I think I found out why we had those bulma-css-variable problems… But not in this pull request.
👍 1
dpschen merged commit eaeddda4e4 into main 2023-02-03 16:34:49 +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
dpschen
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#2997
No description provided.
Delete Branch "dpschen/frontend:feature/move-update-to-app"

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?