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

feat: re-style the keyboard shortcuts menu #996

Merged
konrad merged 3 commits from feat/restyle-shortcuts into main 2021-11-14 16:35:42 +00:00
Conversation 1 Commits 3 Files Changed 1 +29 -6
konrad commented 2021-11-13 21:03:19 +00:00
Owner
Copy Link

Before:

image

After:

image

I think this looks a lot cleaner now.

Before: ![image](/attachments/4177f5b1-8b44-48d1-b419-8dc7f9328541) After: ![image](/attachments/deff49bc-0fe2-4a71-85c5-9c20c1953352) I think this looks a lot cleaner now.
image.png
68 KiB
image.png
56 KiB
konrad added 1 commit 2021-11-13 21:03:19 +00:00
feat: re-style the keyboard shortcuts menu
All checks were successful
continuous-integration/drone/pr Build is passing
Details
03fd7a5c27
frederick commented 2021-11-13 21:21:14 +00:00
Member
Copy Link

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://996-featrestyle-shortcuts--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 konrad! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://996-featrestyle-shortcuts--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 requested changes 2021-11-14 13:28:45 +00:00
src/components/misc/keyboard-shortcuts/index.vue
@ -49,2 +49,3 @@
<style>
<style scoped>
dl {
dpschen commented 2021-11-14 13:24:58 +00:00
Member

Use classes and not tags to reduce specifity and make it possible to change tags without the need to change styles.
Of course we tried to make a good choice here with the tags, but you never know :)

Example:

.shortcut-list {}
.shortcut-title {}
.shortcut-key {}
Use classes and not tags to reduce specifity and make it possible to change tags without the need to change styles. Of course we tried to make a good choice here with the tags, but you never know :) Example: ``` .shortcut-list {} .shortcut-title {} .shortcut-key {} ```
konrad commented 2021-11-14 16:01:01 +00:00
Author
Owner

Changed.

Changed.
👍 1
konrad marked this conversation as resolved
src/components/misc/keyboard-shortcuts/index.vue
@ -53,0 +62,4 @@
margin-bottom: .5rem;
}
.message:not(:last-child) {
dpschen commented 2021-11-14 13:22:05 +00:00
Member

Can .message ever be a :last-child? Seems like there should always be shortcuts below.

Can `.message` ever be a `:last-child`? Seems like there should always be shortcuts below.
konrad commented 2021-11-14 15:59:00 +00:00
Author
Owner

Here it cannot, but the styling from bulma uses .message:not(:last-child). This requires me to use the exact same thing if I want to override it, another possibility would be !important but I think this one is a bit cleaner.

Here it cannot, but the styling from bulma uses `.message:not(:last-child)`. This requires me to use the exact same thing if I want to override it, another possibility would be `!important` but I think this one is a bit cleaner.
dpschen commented 2021-11-14 16:01:36 +00:00
Member

=/ that's ugly.
Okay keep it for now.
I think we should create a message component either way.
Would remove a lot of duplication.
Then we can use own classes and don't need all the bulma stuff.

=/ that's ugly. Okay keep it for now. I think we should create a message component either way. Would remove a lot of duplication. Then we can use own classes and don't need all the bulma stuff.
konrad commented 2021-11-14 16:02:37 +00:00
Author
Owner

I think we should create a message component either way.

Oh yes, definitely. There's even a few variants of it in use already...

> I think we should create a message component either way. Oh yes, definitely. There's even a few variants of it in use already...
dpschen marked this conversation as resolved
src/components/misc/keyboard-shortcuts/index.vue Outdated
@ -53,0 +70,4 @@
padding: .75rem;
}
.keyboard-shortcuts {
dpschen commented 2021-11-14 13:25:47 +00:00
Member

Try to order classes by appearance in the template (makes it easier to follow when the component grows in size).

Try to order classes by appearance in the template (makes it easier to follow when the component grows in size).
konrad commented 2021-11-14 16:02:06 +00:00
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
src/components/misc/keyboard-shortcuts/index.vue
@ -53,0 +71,4 @@
}
.keyboard-shortcuts {
text-align: left;
dpschen commented 2021-11-14 13:28:40 +00:00
Member

This seems like a hack that is needed because the modal.vue currently doesn't provide a variant for this usecase.
Seems like a valid usecase though.
Probably we should redo the modal at some point to remove these kind of hacks.

This seems like a hack that is needed because the `modal.vue` currently doesn't provide a variant for this usecase. Seems like a valid usecase though. Probably we should redo the modal at some point to remove these kind of hacks.
konrad commented 2021-11-14 16:02:57 +00:00
Author
Owner

So this is fine for now?

So this is fine for now?
dpschen commented 2021-11-14 16:32:03 +00:00
Member

Yes! =)

Yes! =)
dpschen marked this conversation as resolved
konrad added 1 commit 2021-11-14 16:00:56 +00:00
chore: use classes instead of elements
All checks were successful
continuous-integration/drone/pr Build is passing
Details
dbeda7dc0e
konrad added 1 commit 2021-11-14 16:02:03 +00:00
chore: reorder styles so they follow the same order as in the markup
All checks were successful
continuous-integration/drone/pr Build is passing
Details
c3cb519c31
dpschen approved these changes 2021-11-14 16:32:33 +00:00
konrad merged commit fcadbc352b into main 2021-11-14 16:35:42 +00:00
konrad deleted branch feat/restyle-shortcuts 2021-11-14 16:35:42 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
dpschen
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
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#996
No description provided.
Delete Branch "feat/restyle-shortcuts"

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?