DN-3604: Follow UvA Design system for all input fields#46
DN-3604: Follow UvA Design system for all input fields#46danielle-devv wants to merge 26 commits intomainfrom
Conversation
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
danielleklaasen
left a comment
danielleklaasen
left a comment
There was a problem hiding this comment.
Looks really good 😄 just some minor comments. Also I noticed there are no links to Storybook in the ticket yet for the testers
| } | ||
| }; | ||
| const handleConfirm = (selectedUsers: UserSearchResult[]) => | ||
| onChange?.(selectionMode === "single" ? selectedUsers[0] : selectedUsers); |
There was a problem hiding this comment.
A little inconsistency in the type usage here, we define UserSearchResult | UserSearchResult[] | null on line 17 of this file, but we don't handle null here anymore like in the removed code. If selectedUsers is empty and you access selectedUsers[0], it returns undefined.
There was a problem hiding this comment.
Good spot! I've added the null check
| )} | ||
| > | ||
| <span className="ui:flex-1 ui:truncate">{item.rendered}</span> | ||
| {isSelected && ( |
There was a problem hiding this comment.
Question, why was this removed again? I think it was added on request of our previous designer.
There was a problem hiding this comment.
I think the checkmark icon comes from another system at UvA that uses that. Right now I changed the colors so that the cell becomes blue when selected. So a visible icon is not necessary anymore for distinction (and it is a nuisance with alignment). Maybe we should ask Carlien for a design for the open selectors?
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
|
I have not managed to make the search input accessible yet: hitting enter does not open the modal. Should this be part of the ticket? |
annesnour03
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
annesnour03
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
danielle-devv
temporarily deployed
to
storybook-pr-46
GitHub Actions
Inactive
annesnour03
requested a deployment
to
storybook-pr-46
GitHub Actions
In progress
| selectionMode = "single", | ||
| "aria-label": ariaLabel = "Search results", | ||
| }: SearchListBoxProps) { | ||
| const [selectedKeys, setSelectedKeys] = useState<Selection>(new Set()); |
There was a problem hiding this comment.
selectedKeys is also being passed as a prop, but is ignored in this component (UserPickerModal passes it). This wrapper always initializes the inner ListBox with a empty set. Previously if you had selected a user and went back to the modal, the user you had selected would show up as selected in the ListBox, this is not the case anymore. Can we address this?
3 participants
I changed some styling for the input elements to match the UvA design.
For the user select I made a new ListBox called SearchListBox that takes a primary and possible secondary value to display in the selector. The default ListBox takes children instead of data, which cannot be styled from there. I don't know if this is the preferred solution though. We could make the data-one the default one since it's currently the only place we use it. Or we could make it an instance outside of the ui package.