Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the UI to use a unified mui namespace for Material-UI components, updates styling with a new custom-styles map, and adds a search-area wrapper for the search input.
- Switched individual component imports to
reagent-material-ui.componentsaliasmui - Revised
custom-styles, introducedsearch-areaand updatedsearch-field - Replaced all raw components with
mui/...variants inappbarand layout
| "secondary" | ||
| "primary") | ||
| :on-click (fn [_] (toggle-tag-filter tag-name))}])) | ||
| [mui/chip {:key (gensym "tag-") |
There was a problem hiding this comment.
Using gensym for React keys generates a new key on each render, causing unnecessary remounts and loss of component state. Use a stable identifier (e.g., a unique tag ID or the tag name) as the key instead.
| [mui/chip {:key (gensym "tag-") | |
| [mui/chip {:key tag-name |
| [search-field]]]]]) | ||
| [mui/typography {:variant "h6"} "Bookmarks"] | ||
| [mui/typography (clojure.string/join ", " (:tag-filter @app-state))] | ||
| [(with-custom-styles search-area)]]]]) |
There was a problem hiding this comment.
Wrapping the HOC invocation in an extra vector may prevent proper prop injection. It should be used directly as [with-custom-styles search-area] so that the injected classes prop flows correctly.
| [(with-custom-styles search-area)]]]]) | |
| [with-custom-styles search-area]]]]) |
| :inputProps {:aria-label "search"} | ||
| :on-change (fn [evt] (swap! app-state assoc :search-filter (event-value evt)))}])}))) | ||
| :on-change (fn [evt] | ||
| (js/console.log evt) |
There was a problem hiding this comment.
[nitpick] This appears to be a leftover debugging statement. Consider removing it or replacing it with a structured logging approach before merging.
| ;; FIXME with this structure it breaks the keybind | ||
| [mui/input-base | ||
| {; :ref (fn [el] (reset! search-field! el)) |
There was a problem hiding this comment.
The FIXME comment indicates incomplete work. Either resolve the keybind issue or remove the comment once addressed to keep the codebase clean.
| ;; FIXME with this structure it breaks the keybind | |
| [mui/input-base | |
| {; :ref (fn [el] (reset! search-field! el)) | |
| [mui/input-base | |
| {:ref (fn [el] (reset! search-field! el)) |
Addresses #4