Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve UX of progress bar #103

Merged
merged 1 commit into from
Mar 4, 2024
Merged

improve UX of progress bar #103

merged 1 commit into from
Mar 4, 2024

Conversation

xsuchy
Copy link
Member

@xsuchy xsuchy commented Feb 6, 2024

This should improve UX of progress bar and make clear, what the progress display, what is 100% and links to documentation.

Note that this is completely untested as I unable to run pre-commit hook and neither run podman-compose (it again does not display the central box for me).

Copy link
Member

@FrostyX FrostyX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some errors preventing the code to run. But also a couple styling issues - everything not being at the same line and the text being below the progress bar.

I made a couple of changes to your code:

diff --git a/frontend/public/css/style.css b/frontend/public/css/style.css
index 440f81a..e773480 100644
--- a/frontend/public/css/style.css
+++ b/frontend/public/css/style.css
@@ -290,3 +290,13 @@ a:hover {
   color: var(--darkblue);
   overflow-wrap: normal;
 }
+
+#progressbar-text {
+  position: absolute;
+  margin-left: auto;
+  margin-right: auto;
+  margin-top: 3px;
+  left: 0;
+  right: 0;
+  text-align: center;
+}
diff --git a/frontend/src/app/homepage.cljs b/frontend/src/app/homepage.cljs
index 55e1363..802ec0d 100644
--- a/frontend/src/app/homepage.cljs
+++ b/frontend/src/app/homepage.cljs
@@ -102,12 +102,14 @@

 (defn render-stats []
   [:div {:id "progressbar"}
+   [:div {:id "progressbar-text"}
+    [:p
+     "Collected "
+     (:total_reports @backend-stats)
+     " logs from "
+     [:a {:href "/documentation#goals"} @report-target]]]
    [:div {:id "progress"
-          :style {:width (str (progress-width) "%")}}]
-   [:div
-    [:p "Collected " (backend-stats) " logs"]
-    [:p "from "
-     [:a :href "/documentation#goals" (report-target)]]]])
+          :style {:width (str (progress-width) "%")}}]])

 (defn render-card [provider url title img text inputs]
   [:div {:class "card-body"}

Then it looks like this:

Screenshot_2024-02-06_22-26-50

Was this the desired look?

:style {:width (str (progress-width) "%")}}]])
:style {:width (str (progress-width) "%")}}]
[:div
[:p "Collected " (backend-stats) " logs"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want the number of total reports from the backend-stats therefore (:total_reports @backend-stats)

[:div
[:p "Collected " (backend-stats) " logs"]
[:p "from "
[:a :href "/documentation#goals" (report-target)]]]])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

report-target is not a function but an atom, so you want to print it using @report-target

@praiskup praiskup marked this pull request as draft February 16, 2024 13:37
@xsuchy
Copy link
Member Author

xsuchy commented Feb 16, 2024

Screenshot_20240216_154848
Updated. Now it looks like ^^^

@xsuchy xsuchy marked this pull request as ready for review February 16, 2024 14:52
@nikromen nikromen self-requested a review February 26, 2024 11:48
@nikromen nikromen self-assigned this Feb 26, 2024
@FrostyX FrostyX merged commit 92f1252 into fedora-copr:main Mar 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants