Feature/645 Add units in map screen #654
Conversation
ismaelbej
force-pushed
the
feature/635-source-type-template
branch
2 times, most recently
from
ced16a7 to
67b867c
Compare
ismaelbej
force-pushed
the
feature/645-add-units-in-map-screen
branch
from
3a2246e to
58e81ca
Compare
There was a problem hiding this comment.
Looks good! Check the comments for improvements suggestions. Seeing the amount of string manipulation and building in the UI, it may be beneficial to add the cuerdas library or something similar. But it's probably a bit late in the iteration.
| [{:keys [capacity-unit demand-unit] :as props} {:keys [name change satisfied-demand] :as provider}] | ||
| (let [action (:action change)] | ||
| [:div | ||
| [:div {:class-name "section changeset-row" |
There was a problem hiding this comment.
For fixed CSS classes, the notation on the keyword is preferred. Eg. :div.section.changeset-row in this case.
| (str (get action-texts action) | ||
| " " | ||
| (utils/format-number (:capacity change)) | ||
| " " | ||
| capacity-unit | ||
| " to serve " | ||
| (utils/format-number satisfied-demand) | ||
| " " | ||
| demand-unit | ||
| (when (pos? (:investment change)) | ||
| (str " at " (utils/format-currency (:investment change)))))]]]] |
There was a problem hiding this comment.
I'd extract this into a function and try to keep the action string prefix (action-texts) together with the rest of the interpolation. That way you can easily spot if the whole sentence makes sense. Eg.
(defn- action-string
[{:keys [action capacity capacity-unit demand-unit investment satisfied-demand]}]
(let [formatted-capacity (format-number-with-unit capacity capacity-unit)
formatted-demand (format-number-with-unit satisfied-demand demand-unit)
formatted-investment (when (pos? investment) (str " at " (utils/format-currency investment)))]
(case action
"create-provider" (str "New build with " formatted-capacity " to serve " formatted-demand formatted-investment)
"upgrade-provider" (str "Upgraded and increased capacity on " formatted-capacity " to serve " formatted-demand formatted-investment)
"increase-provider" (str "Increased capacity on " formatted-capacity " to serve " formatted-demand formatted-investment)))There is a bit of repetition, but I think it's worth it and makes it easier to change the phrase for one type of change if necessary. The investment part is still "hard-coded" and could even be extracted further, or maybe you could enumerate all 6 cases separately (the 3 actions, with or without investment).
Also includes #644 and #651.