fix: namespace new buttons on mobile #1262

Merged
dpschen merged 10 commits from fix/new-buttons into main 2022-01-05 12:46:34 +00:00
Owner

Before:

image

After:

image

Before: ![image](/attachments/7626e28d-8a13-4f92-b162-697676f765c7) After: ![image](/attachments/5331af47-4887-4ba5-98d7-ee70311e20d7)
konrad added 1 commit 2021-12-29 20:30:01 +00:00
continuous-integration/drone/pr Build is failing Details
cde2f198d0
fix: namespace new buttons on mobile
Member

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1262-fixnew-buttons--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://1262-fixnew-buttons--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 added 1 commit 2021-12-30 12:37:35 +00:00
continuous-integration/drone/pr Build was killed Details
9aeb9e7349
fix: tests
konrad added 1 commit 2021-12-30 12:37:54 +00:00
continuous-integration/drone/pr Build is passing Details
4006822089
fix: remove .only test modifier
dpschen requested changes 2021-12-30 13:37:31 +00:00
@ -73,3 +73,3 @@
.should('contain', newNamespaceName)
.should('not.contain', newNamespaces[0].title)
cy.get('.content.namespaces-list')
cy.get('.content')

Use a v-cy anchor for this:

I think we should reduce the use of the content class from bulma.
By bulma it is meant as a wrapper class for blog like / prose content.

Use a `v-cy` anchor for this: I think we should reduce the use of the content class from bulma. By bulma it is meant as a wrapper class for blog like / prose content.
Author
Owner

Done.

I think we should reduce the use of the content class from bulma.
By bulma it is meant as a wrapper class for blog like / prose content.

Agreed.

Done. > I think we should reduce the use of the content class from bulma. By bulma it is meant as a wrapper class for blog like / prose content. Agreed.
konrad marked this conversation as resolved
@ -1,4 +0,0 @@
// FIXME: used in navigation.vue and in ListNamespaces.vue

Nice! 🔥

Nice! 🔥
konrad marked this conversation as resolved
@ -13,0 +5,4 @@
{{ $t('namespace.showArchived') }}
</fancycheckbox>
<div>

This div doesn't add a semantics and I can't find styles applied. What is it for?

This `div` doesn't add a semantics and I can't find styles applied. What is it for?
Author
Owner

It's used to put the two buttons on the right side, since I applied justify-content: space-between to the container one level above.

It's used to put the two buttons on the right side, since I applied `justify-content: space-between` to the container one level above.

How about:

.new-namespace:first-child {
    margin-left: auto;
}

Or using a ul with li to add semantics.

How about: ```css .new-namespace:first-child { margin-left: auto; } ``` Or using a ul with li to add semantics.
Author
Owner

That's a really nice trick! I've used an ml-auto class instead though, the :first-child selector didn't work.

That's a really nice trick! I've used an `ml-auto` class instead though, the `:first-child` selector didn't work.
konrad marked this conversation as resolved
@ -148,0 +138,4 @@
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: .5rem;

The elements below have already a margin top. Can we remove it here?

The elements below have already a margin top. Can we remove it here?
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -148,0 +147,4 @@
> * {
@media screen and (max-width: $tablet) {
margin-bottom: .5rem;

use gap

use `gap`
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -148,0 +151,4 @@
}
}
.new-namespace {

unindent class (no need because it's scoped).

unindent class (no need because it's scoped).
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -148,0 +152,4 @@
}
.new-namespace {
margin-left: 1rem;

Scope this for min-width $tablet so that we don't need an overwrite for smaller devices (mobile first)

Scope this for `min-width $tablet` so that we don't need an overwrite for smaller devices (mobile first)
Author
Owner

Done.

I had to override the width though.

Done. I had to override the width though.
konrad marked this conversation as resolved
konrad added 1 commit 2021-12-30 15:49:10 +00:00
continuous-integration/drone/pr Build is passing Details
1a3fc822bb
chore: use v-cy for test
konrad added 3 commits 2021-12-30 16:02:30 +00:00
konrad added 1 commit 2021-12-30 16:05:25 +00:00
continuous-integration/drone/pr Build is failing Details
3d8b473080
chore: use gap
konrad added 1 commit 2021-12-30 16:08:13 +00:00
continuous-integration/drone/pr Build is passing Details
74af089571
chore: use margin left to remove extra div
dpschen force-pushed fix/new-buttons from 79d5c65e1d to 5b6d8f5527 2022-01-05 10:57:45 +00:00 Compare
dpschen force-pushed fix/new-buttons from 5b6d8f5527 to 8a95bb7422 2022-01-05 12:33:02 +00:00 Compare
dpschen approved these changes 2022-01-05 12:45:54 +00:00
dpschen merged commit c618b7e0b6 into main 2022-01-05 12:46:34 +00:00
This repo is archived. You cannot comment on pull requests.
No description provided.