feat(frontend): improve open tables drawer design #1404

Bettelstab · 2024-07-11T22:40:34Z

🍰 Pullrequest

Styles the open tables drawer according to Figma. Also adds messages in case of no open tables or no search results.

Also fixes these warnings:

Issues

fixes 🚀 [Feature] Frontend: Optimize openRoom Sidebar Design #1171

Todo

Set border color of search field depending on theme
Check colors - there should be a contrast on mobile?
Participants like in Figma (https://www.figma.com/design/nkCyyDFx7fGeBnguwpardz/Dreammall-pr%C3%A4si?node-id=3359-32946&m=dev)
Ability to drag down on mobile would be nice. Should work, but doesn't ([Bug Report] Navigation drawer with bottom not opened on swipe from bottom vuetifyjs/vuetify#10713) - Let's do it later (🚀 [Feature] Swipe gestures #1456)
Tests

roschaefer

I will request changes for the screenshot below:

frontend/src/components/menu/TopMenu.vue

roschaefer · 2024-07-21T17:57:00Z

frontend/src/components/menu/TopMenu.vue

@@ -61,6 +65,6 @@ const toggleDrawer = () => {
 .top-menu {
  position: sticky;
  top: 0;
-  z-index: 1000;
+  z-index: 1008;


There must be a problem if it is necessary to change z-index values like that.

I agree. Just had a quick look at https://www.smashingmagazine.com/2021/02/css-z-index-large-projects and the idea makes sense to me. We are dealing with Vuetify values here, I'll check if there is any way to get those values.

I simplified the structure and got rid of that specific z-index entirely. Was like that before because of requirements which changed.

roschaefer · 2024-07-21T17:57:36Z

frontend/src/components/menu/__snapshots__/BottomMenu.test.ts.snap

+
+    <nav
+      class="v-navigation-drawer v-navigation-drawer--bottom v-navigation-drawer--temporary v-theme--light v-navigation-drawer--mobile menu-drawer px-4 d-md-none"
+      data-v-cf2c9067=""


What is data-v-cf2c9067?

This is what makes scoped CSS work. The tag is afaik generated from the file name or path.

🤦 I was reviewing generated code. Of course, this is how it works with scoped CSS.

roschaefer · 2024-07-21T17:57:54Z

frontend/src/components/menu/__snapshots__/BottomMenu.test.ts.snap

+    <nav
+      class="v-navigation-drawer v-navigation-drawer--bottom v-navigation-drawer--temporary v-theme--light v-navigation-drawer--mobile menu-drawer px-4 d-md-none"
+      data-v-cf2c9067=""
+      style="bottom: 0px; z-index: 1004; transform: translateY(110%); position: fixed; transition: none !important; left: 0px; width: calc(100% - 0px - 0px);"


Why do we need style attribute here?

It's generated by Vuetify unfortunately.

Again generated code. But what the heck, Vuetify, style attributes, seriously? They have the highest specificity: https://2019.wattenberger.com/blog/css-cascade#specificity

roschaefer · 2024-07-21T17:59:50Z

frontend/src/components/menu/__snapshots__/TopMenu.test.ts.snap

      data-v-cf2c9067=""
-      style="right: 0px; z-index: 1004; transform: translateX(110%); position: fixed; transition: none !important; height: calc(100% - 70px - 0px); top: 70px; bottom: 0px;"
+      style="right: 0px; z-index: 1004; transform: translateX(110%); position: fixed; height: calc(100% - 70px - 0px); top: 70px; bottom: 0px; transition: none !important;"


Is the order important here?

I have no explanation why it changed...

Sorry for that, if it's generated, I pretty much don't care about the order here.

roschaefer · 2024-07-21T18:01:11Z

frontend/src/components/menu/__snapshots__/TopMenu.test.ts.snap

@@ -872,7 +872,7 @@ exports[`TopMenu > renders 1`] = `

                  <i
                    aria-hidden="false"
-                    aria-label="$vuetify.input.clear"
+                    aria-label="Löschen"


I can see a $t('tablesDrawer.noTables') further down. So we're already translating the app, why not translate this, too?

We are translating this, of course. Apparently the Vuetify internal dictionary entries are resolved for the snapshots. That's a strange behaviour and could lead to problems in CI. Good catch.
Why does it appear now: I corrected the keys in the locales files.

Yes! It's good that it's translated now 🙂 the problems in unit testing is another story.

roschaefer · 2024-07-21T18:04:47Z

frontend/src/components/tablesDrawer/TablesDrawer.vue

+  top: 0 !important;
+  z-index: 1006 !important;
+  width: 308px;
+  height: 100% !important;
+  padding-top: 70px;
+  background: var(--v-sidebar-background) !important;
+}
+
+@media #{map-get($display-breakpoints, 'sm-and-down')} {
+  .menu-drawer {
+    left: var(--sides) !important;
+    z-index: 2000 !important;
+    width: calc(100% - (2 * var(--sides))) !important;


The sheer amount of !importants in this block indicate a specificity problem.

Vuetify components that we need to override :/

Vuetify components that we need to override :/

This is exactly the selling point of Tailwind. No more specificity wars. Every component framework that is built on Tailwind is using the same level of specificity. Every template code that we copy+paste will look the same in our application as it does in a sandbox. The nesting template will only pass in CSS that is not specified in the inner template.

Problems solved. Once. And. For. All.

roschaefer · 2024-07-21T18:06:32Z

frontend/src/locales/de.json

@@ -1,6 +1,6 @@
 {
  "$vuetify": {
-    "input:": {
+    "input": {
      "clear": "Löschen"


Why was this not used?

What do you mean? It's used in the search field.

roschaefer · 2024-07-21T18:10:38Z

frontend/src/components/tablesDrawer/TableList.vue

-      </v-list-item>
-    </v-list>
-  </v-card>
+  <ul class="list">


I like that we're using plain HTML when possible 👍

Bettelstab · 2024-07-22T10:22:35Z

@roschaefer
Thank you for the valuable feedback! I'll address some of the issues you mentioned. Your main point: There is the plan to move the Light/Dark switch from where it is to some settings menu, therefore we decided to ignore this problem.

Bettelstab · 2024-07-22T11:58:01Z

@roschaefer
Please have another look :) Guess we should create an issue for the vuetify translation thing

roschaefer

We could give the light/dark switch a white outline, just like the other buttons.

I will hit approve here to get things going though.

I'm afraid this won't be the last PR where I will complain about vuetify.

roschaefer · 2024-07-23T13:54:42Z

frontend/src/components/menu/TopMenu.vue

-  position: sticky;
-  top: 0;
-  z-index: 1008;
+.hide-on-mobile {


This is such a utility class. It shouldn't go into scoped. I think in tailwind it would be invisible md:visible.

Yes, I totally agree and will make it global with a better name the next time I'll use it. No, visibility is something different and takes rendering time. md:hidden would be right, I guess.

Bettelstab force-pushed the sidebar-style branch from 3e9b7ff to 1b6062d Compare July 11, 2024 23:17

Bettelstab self-assigned this Jul 12, 2024

Bettelstab force-pushed the sidebar-style branch from 1b6062d to bb168cc Compare July 15, 2024 21:50

Bettelstab added 10 commits July 19, 2024 14:37

Improve sidebar design, show messages for empty table list

6ad8c40

Adapt mobile tables drawer design

4d230dd

Fix linting

e749baa

Fix locales

e844f8c

Update snapshots

f43ba91

Fix TableList test

a0c2962

Don't render left tables drawer on small screens

dd81cd3

Adapt table info to Figma drafts

9df5e5e

Use correct border-radius and background color

31557d3

Remove some opacity

6f0dc02

Bettelstab force-pushed the sidebar-style branch from bb168cc to 6f0dc02 Compare July 19, 2024 12:37

Bettelstab added 3 commits July 19, 2024 16:02

Fix search field border and background color

6a2287f

Fix linting

cac57ab

Update snapshots

1ecf5be

Bettelstab marked this pull request as ready for review July 19, 2024 14:08

roschaefer requested changes Jul 21, 2024

View reviewed changes

Simplify top menu dom

2612a7a

Update snapshots

0ad62a9

roschaefer approved these changes Jul 23, 2024

View reviewed changes

Merge branch 'master' into sidebar-style

80bff93

Bettelstab enabled auto-merge (squash) July 23, 2024 18:49

Bettelstab merged commit d92018a into master Jul 23, 2024
35 checks passed

mahula deleted the sidebar-style branch July 25, 2024 07:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): improve open tables drawer design #1404

feat(frontend): improve open tables drawer design #1404

Bettelstab commented Jul 11, 2024 •

edited

Loading

roschaefer left a comment

roschaefer Jul 21, 2024

Bettelstab Jul 22, 2024

Bettelstab Jul 22, 2024 •

edited

Loading

roschaefer Jul 21, 2024

Bettelstab Jul 21, 2024

roschaefer Jul 23, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 21, 2024

roschaefer Jul 23, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 21, 2024

roschaefer Jul 23, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 22, 2024

roschaefer Jul 23, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 22, 2024

roschaefer Jul 23, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 21, 2024

roschaefer Jul 21, 2024

Bettelstab Jul 21, 2024

Bettelstab commented Jul 22, 2024 •

edited

Loading

Bettelstab commented Jul 22, 2024

roschaefer left a comment

roschaefer Jul 23, 2024

Bettelstab Jul 23, 2024 •

edited

Loading

feat(frontend): improve open tables drawer design #1404

feat(frontend): improve open tables drawer design #1404

Conversation

Bettelstab commented Jul 11, 2024 • edited Loading

🍰 Pullrequest

Issues

Todo

roschaefer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bettelstab Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bettelstab commented Jul 22, 2024 • edited Loading

Bettelstab commented Jul 22, 2024

roschaefer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bettelstab Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Bettelstab commented Jul 11, 2024 •

edited

Loading

Bettelstab Jul 22, 2024 •

edited

Loading

Bettelstab commented Jul 22, 2024 •

edited

Loading

Bettelstab Jul 23, 2024 •

edited

Loading