feat: restyle unauthenticated screens #1103
No reviewers
dpschen
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
No Assignees
3 Participants
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#1103
Loading…
Reference in New Issue
No description provided.
Delete Branch "feature/login-pages"
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?
I wanted to give the no-auth screens a new look. Here's what I ended up with:
The image is something we could change with every release.
Hi konrad!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://1103-featurelogin-pages--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!
Login wrapper collides with llama
The Login / Register buttons are confusing:
When you are on login and click on "Register": it seems that only the position is toggled.
=> The action on the button, the color and position changes for the buttons on the two routes but the label stays the same.
Below screenshots from fb and twitter for inspiration.
(I know this was the case before but since we are trying improve UX here I guess this makes sense to add)
The success messages feels like a duplication of the content of the api-config.
Maybe just changing the value is enough? Or use a success notification?
Hope I was not to picky. I'm trying to align our coding style.
If too annoying please tell me =)
@ -17,0 +19,4 @@.message-wrapper {border-radius: $radius;background: var(--white);overflow: hidden;I don't get the need for the new wrapper.
What's the
overflowfor?That's because the message background is transparent. When I put the message on something with a background, it would shine through. The overlay puts a white background behind it so the message background color is always the same.
I just realized I don't need the overflow so I just removed it.
@ -14,2 +24,3 @@import Logo from '@/components/home/Logo.vue'import message from '@/components/misc/message'import Message from '@/components/misc/message'import Legal from '../../components/misc/legal'Use
@Done.
@ -26,1 +37,4 @@min-height: 100vh;display: flex;flex-direction: column;justify-content: center;Super picky: use
place-items: center(I think this is replaced with the same after parsed :D)Oh, that sounds like a nice property, never heard of it before.
@ -32,1 +47,3 @@margin: 0 auto;min-height: 60vh;display: flex;background: var(--white);Picky:
Use
background-color=> more specific, doesn't change other values if they are set (they are not in this case). Makes it easier to overwrite if necessary.@ -33,0 +49,4 @@background: var(--white);border-radius: $radius;box-shadow: var(--shadow-md);overflow: hidden;Why overflow: hidden?
To prevent the background from overflowing in the (rounded) corners.
@ -33,0 +51,4 @@box-shadow: var(--shadow-md);overflow: hidden;@media screen and (max-width: $desktop) {Use mobile first:
=> no need to reset the border-radius for mobile
@ -33,0 +58,4 @@.image {width: 60%;background: url('@/assets/no-auth-image.jpg') no-repeat bottom;Use mobile first:
Compress image and load with something like https://github.com/JonasKruckenberg/imagetools
With the vite wrapper and use it for other images as well?
Yes. But maybe let's move this to a new issue.
yeah I think we should.
@ -34,0 +81,4 @@background: rgba(0, 0, 0, .2);display: flex;flex-direction: column;justify-content: end;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: autoorjustify-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
endso I guess it's still better to useflex-end.=> Maybe support is better, and it seems that postcss-preset-env / autoprefixer doesn't change this automatically.
Picky: same for background as above.
Moved this to an after pseudo class.
@ -34,0 +89,4 @@}.content {width: 50%;If
.imageis either 40% or 60% why is this 50% wide? Also use mobile first for the width value.Change it so everything is always 50%. That seems to work great.
I'm not quite sure why but it does not work when I only use
min-width: $desktop.The default width of a flex child is auto as far as i know.
@ -34,3 +100,4 @@}}.logo {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
You mean aligned in the middle like this?
Okay wow I can't put images in a comment here, here's the screenshot I wanted to post:
I'm also not really happy with this. Let's keep it for now how it is.
@ -34,4 +101,4 @@}.logo {color: var(--logo-text-color);Color seems to be applied twice (from inside the component aswell).
@ -36,3 +103,4 @@.logo {color: var(--logo-text-color);max-width: 100%;margin: 1rem 0;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.hmm it looks like this isn't transformed. But it definitely is a nice propery.
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
@ -39,0 +106,4 @@margin: 1rem 0;}.message {Add class to Message component and use that.
=> By using a class inside the component we would create an implicit dependency.
It looks like the class here wasn't used anywhere. I've removed it.
@ -25,0 +4,4 @@<message variant="success" class="has-text-centered" v-if="confirmedEmailSuccess">{{ $t('user.auth.confirmEmailSuccess') }}</message><api-config @foundApi="hasApiUrl = true"/>There is no
<api-config>in the Register and Reset-password route. Should it be there aswell?I think it should. I've moved the
<api-config/>component to the wrapper and the logo as well in the process. Should be better now.@ -79,1 +58,3 @@</div><div class="field is-grouped login-buttons"><div class="control is-expanded"><x-buttonMake login button larger
=> "hard" to tap on mobile if you keep in mind that this is the primary action of the page.
Would like to do that in #1104 while changing the register link wording you mentioned earlier.
@ -98,3 +79,2 @@</div><legal/><message variant="danger" v-if="errorMessage">The error message should be on top => can't see on small screens without scrolling
Done.
@ -64,0 +52,4 @@</message></form><div class="has-text-centered" v-if="successMessage"><message variant="success">Move error and succes to top here aswell.
@ -44,3 +32,2 @@</x-button></div><Legal /><message variant="danger" v-if="errorMsg">Move error messages here to top aswell.
The one column variant might gain from having a smaller max-width.
Oh right. Now hiding it on smaller screens where it would collide.
Makes sense. I'd like to move changing this to #1104 since this PR is mostly about styling while #1104 is about ux.
I like the idea of a notification.
So basically more horizontal margin?
Auto margin and a max-width.
I think it should be "Using Vikunja installation at" [...]" (instead of "on") in for the notification aswell as the api-config. (unsure am not native ;) )
The image still seems to be the large version (according to the gitea size value).
@ -27,0 +52,4 @@place-items: center;@media screen and (min-width: $fullhd) {background: url('@/assets/llama.svg?url') no-repeat bottom left fixed;Just checked this: The llama is so nice, it's too bad it's not visivle on mobile. How about adding a padding-bottom for mobile?
done!
Done.
I think it makes sense though. Changed it.
Added the compressed version you sent me, I think it should be fine now?
@ -33,0 +71,4 @@}.image {width: 50%;Move these rules to the
@media screen and (min-width: $tablet)media query=> makes it clearer that it just appies there.
Move media queries before
&::afterand> *=> those are styling other elements
=> move styles together that style the same stuff
If you change
&::afterin&::beforeyou can remove the z-index if you replacez-index: 20;withposition: relative(this creates a new stacking context.=> less complexity with managing z-index.
Done!
@ -34,0 +106,4 @@}}.content {Use mobile first:
Done!