Labels
No labels
bug
duplicate
enhancement
help wanted
high priority
invalid
L
low priority
M
medium priority
needs refinement
question
S
UX research
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: local-it/mitgliederverwaltung#166
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/152_sorting_default_fields"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description of the implemented changes
The changes were:
What has been changed?
Definition of Done
Code Quality
Accessibility
Testing
Additional Notes
9b64e35cceto3b3ef796a4I implemented the Sort function as LiveComponent which we could reuse.
I added an issue for the aria-sort label which I then left out: #174
I kept the TableComponents file because this sort button is still used in users. As you are currently working on users - members, I thought it would be better to take care of userslist afterwards?
WIP: Sorting header for members list closes#152to Sorting header for members list closes#1525a17a9e929to78cbd3cf80If the rows are sorted by a specific column the column header shows a single arrow in the sorting direction. The other rows show a double arrow (up and down). If I sort for another column the double arrow switches into a single arrow, but the previous column header arrow keeps the same. So it's not possible to see which column is the column sorted by. It would be good to reset the sorting arrows of the previous columns. Maybe something more obvious to highlight which is the current sort column would be nice too.
78cbd3cf80toe8d440f74fLooks very good 🚀
I added many small things. Main issues to fix are the search + sort bug and the resetting of previous sort arrows.
@ -0,0 +2,4 @@@moduledoc """Sort Header that can be used as column header and sorts a table:Props:- field: atom() # Ash‑Field for sortingambiguous Unicode character
-in comment.@ -0,0 +3,4 @@Sort Header that can be used as column header and sorts a table:Props:- field: atom() # Ash‑Field for sorting- label: string() # Column Heading (can be aan heex templyte)typo
templyte@ -0,0 +48,4 @@end# -------------------------------------------------# Hilfsfunktionen für ARIA‑Attribute & Icon‑SVGambiguous Unicode character - in comment.
@ -40,2 +16,2 @@|> assign(:query, q)|> assign(:members, members)}# We call handle params to use the query from the URL{:noreply, socket} = handle_params(params, nil, socket)https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html
"The life-cycle is: mount/3 -> handle_params/3 -> render/1"
Therefore handle_params is called twice, in your mount/3 and afterwards by the LiveView. Each handdle_params loads all the members. Is this line necessary at all? If I just remove it it still works for me.
@ -111,0 +93,4 @@}# "/members" is the path you defined in router.exnew_path = "/members?" <> URI.encode_query(query_params)Best practice: instead of hardcoded paths, render path dynamically:
Also the
~psigil could be used for creating a path.I tried that already before and did not find a simple solution for getting the current path. So I would keep that static for now.
@ -111,0 +106,4 @@# Function to handle search@impl truedef handle_info({:search_changed, q}, socket) doThis function ignores the sorting. This leads to reset the sorting while searching.
To avoid duplicate code I would recommend to integrate the search logic into
load_members.@ -0,0 +9,4 @@assert view |> element("[data-testid='first_name']")endendMaybe some more tests like:
@ -78,0 +79,4 @@conn = conn_with_oidc_user(conn){:ok, view, _html} = live(conn, "/members")# The component data test ids are built as "<field>"ambiguous Unicode character
-in comment.@ -78,0 +87,4 @@# The LiveView pushes a patch with the new query paramsassert_patch(view, "/members?sort_field=email&sort_order=asc")ambiguous Unicode character
-in comment.@ -112,0 +81,4 @@active_id = :"sort_#{new_field}"# Update the SortHeader tosend_update(MvWeb.Components.SortHeaderComponent,@moritz wrote in #166 (comment):
maybe something like this could work to reset the old header component:
Sorting header for members list closes#152to Sorting header for members list closes#152 #175Sorting header for members list closes#152 #175to Sorting header for members list closes #152 #1754633a4610bto660d587cc1660d587cc1to8104451d35Looks very good 💯