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

feat: restyle unauthenticated screens #1103

Merged
dpschen merged 29 commits from feature/login-pages into main 2021-12-12 16:40:14 +00:00
Conversation 12 Commits 29 Files Changed 10 +10 -11
1 changed files with 10 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3ab2efb32c - Show all commits

21
src/components/misc/no-auth-wrapper.vue
View File

@ -77,11 +77,20 @@ useTitle(() => title.value)
flex-direction: column;
justify-content: flex-end;
@media screen and (max-width: $tablet) {
display: none;
}
@media screen and (min-width: $tablet) {
konrad marked this conversation as resolved Outdated
dpschen commented 2021-12-09 17:25:21 +00:00
Outdated
Review
Copy Link

Since the image is defined via CSS:
Not sure why the overlay is needed at all:
If the image uses flex itself you could position the title and message directly as a child.

Title could have: margin-top: auto or justify-self: flex-end. I think both should work.

EDIT:
I just saw that the overlay is used for the background-overlay. But the same could be archieved without DOM impact / new element by styling a ::after.

I got a warning for end so I guess it's still better to use flex-end.
=> Maybe support is better, and it seems that postcss-preset-env / autoprefixer doesn't change this automatically.

Picky: same for background as above.

Since the image is defined via CSS: Not sure why the overlay is needed at all: If the image uses flex itself you could position the title and message directly as a child. Title could have: `margin-top: auto` or `justify-self: flex-end`. I think both should work. **EDIT:** I just saw that the overlay is used for the background-overlay. But the same could be archieved without DOM impact / new element by styling a `::after`. I got a warning for `end` so I guess it's still better to use `flex-end`. => Maybe support is better, and it seems that postcss-preset-env / autoprefixer doesn't change this automatically. Picky: same for background as above.
konrad commented 2021-12-11 19:51:15 +00:00
Outdated
Review
Copy Link

Moved this to an after pseudo class.

Moved this to an after pseudo class.
background: url('@/assets/no-auth-image.jpg') no-repeat bottom/cover;
position: relative;
}
&.has-message {
justify-content: space-between;
}
dpschen marked this conversation as resolved Outdated
dpschen commented 2021-12-09 17:58:49 +00:00
Outdated
Review
Copy Link

If .image is either 40% or 60% why is this 50% wide? Also use mobile first for the width value.

If `.image` is either 40% or 60% why is this 50% wide? Also use mobile first for the width value.
konrad commented 2021-12-11 17:37:23 +00:00
Outdated
Review
Copy Link

Change it so everything is always 50%. That seems to work great.

Also use mobile first for the width value.

I'm not quite sure why but it does not work when I only use min-width: $desktop.

Change it so everything is always 50%. That seems to work great. > Also use mobile first for the width value. I'm not quite sure why but it does not work when I only use `min-width: $desktop`.
dpschen commented 2021-12-12 15:16:44 +00:00
Outdated
Review
Copy Link

The default width of a flex child is auto as far as i know.

The default width of a flex child is auto as far as i know.
&::after {
&::before {
content: '';
position: absolute;
top: 0;
@ -89,19 +98,9 @@ useTitle(() => title.value)
right: 0;
bottom: 0;
background-color: rgba(0, 0, 0, .2);
z-index: 10;
}
> * {
dpschen marked this conversation as resolved Outdated
dpschen commented 2021-12-09 18:17:56 +00:00
Outdated
Review
Copy Link

On desktop:
Maybe the logo should be not centered but left aligned with the content below.
=> Might make the Title easier to read because there is more room between those two headlines

On desktop: Maybe the logo should be not centered but left aligned with the content below. => Might make the Title easier to read because there is more room between those two headlines
konrad commented 2021-12-11 19:34:06 +00:00
Outdated
Review
Copy Link

You mean aligned in the middle like this?

You mean aligned in the middle like this?
konrad commented 2021-12-11 19:35:38 +00:00
Outdated
Review
Copy Link

Okay wow I can't put images in a comment here, here's the screenshot I wanted to post:

Okay wow I can't put images in a comment here, here's the screenshot I wanted to post: ![](https://kolaente.dev/attachments/1847041a-d987-46e1-9d70-99c5745e8306)
dpschen commented 2021-12-12 15:15:44 +00:00
Outdated
Review
Copy Link

I'm also not really happy with this. Let's keep it for now how it is.

I'm also not really happy with this. Let's keep it for now how it is.
z-index: 20;
}
@media screen and (max-width: $tablet) {
display: none;
}
@media screen and (min-width: $tablet) {
background: url('@/assets/no-auth-image.jpg') no-repeat bottom/cover;
position: relative;
konrad marked this conversation as resolved Outdated
dpschen commented 2021-12-09 17:59:46 +00:00
Outdated
Review
Copy Link

Color seems to be applied twice (from inside the component aswell).

Color seems to be applied twice (from inside the component aswell).
}
}
konrad marked this conversation as resolved Outdated
dpschen commented 2021-12-09 18:01:28 +00:00
Outdated
Review
Copy Link

Picky: new css love: use margin-block: 1rem; => transformed by postcss-preset-env. Reason: you don't want to define the inline (left, right) value, but do it here as a sideeffect.

Picky: new css love: use `margin-block: 1rem;` => transformed by postcss-preset-env. Reason: you don't want to define the inline (left, right) value, but do it here as a sideeffect.
konrad commented 2021-12-11 19:52:42 +00:00
Outdated
Review
Copy Link

hmm it looks like this isn't transformed. But it definitely is a nice propery.

hmm it looks like this isn't transformed. But it definitely is a nice propery.
dpschen commented 2021-12-12 14:45:31 +00:00
Outdated
Review
Copy Link

I checked this again.

I think that the browsers that we want to support according to our browserslist all support it natively so there is no need for postcss-preset-env to transform it =)

Because it does do that via postcss-logical

I checked this again. I think that the browsers that we want to support according to our browserslist all support it natively so there is no need for postcss-preset-env to transform it =) Because it does do that via [postcss-logical](https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-logical)
👍 1