[ENG-293] Tag assign mode #2462
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
[ENG-293] Tag assign mode
…s state on keypress
There was a problem hiding this comment.
noticed that a lot of the common.json files have english in them despite not being english files? also minor but naming convention for file_two/file_zero is rather confusing, especially because it seems their contents are the same for each language (i might just be misunderstanding the purpose though)
There was a problem hiding this comment.
leaving a comment just on this file but appears to be a common issue across almost all of them
| @@ -118,11 +119,28 @@ export const useExplorerTopBarOptions = () => { | |||
| showAtResolution: 'xl:flex' | |||
| }, | |||
| { | |||
| toolTipLabel: t('key_manager'), | |||
There was a problem hiding this comment.
is there a reason this line was removed in favour of line 122?
| return ( | ||
| <button | ||
| className={clsx('group flex items-center gap-1 rounded-lg border px-2.5 py-0.5', { | ||
| ['border-gray-500 bg-gray-500']: !isAwaitingKeyPress && isDark, | ||
| ['border-blue-400 bg-blue-800']: isAwaitingKeyPress && isDark, | ||
| ['border-blue-500 bg-blue-200']: isAwaitingKeyPress && !isDark | ||
| })} | ||
| ref={buttonRef} | ||
| onClick={onClick} | ||
| aria-live={isAwaitingKeyPress ? 'assertive' : 'off'} | ||
| aria-label={ | ||
| isAwaitingKeyPress | ||
| ? `Type a number to map it to the "${tag.name}" tag. Press escape to cancel.` | ||
| : undefined | ||
| } | ||
| > | ||
| <Circle | ||
| fill={tag.color ?? 'grey'} | ||
| weight="fill" | ||
| alt="" | ||
| aria-hidden | ||
| className="size-3" | ||
| /> | ||
| <span className="max-w-xs truncate py-0.5 text-sm font-semibold text-ink-dull"> | ||
| {tag.name} | ||
| </span> | ||
|
|
||
| {assignKey && <Shortcut chars={keybind([], [assignKey])} />} | ||
| </button> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
the buttons look a bit off, especially selected and against the light blue background. Just off the top of my head:
- removing the semibold text as its rather jarring, maybe a weight 300-500 (whatever works)
- fixing the shape/colour of the assigned key shape, as the outline of the key is only visible against the blue background rn
- possibly changing the click effect. maybe scale would work, but i could see how this could be confusing from a ux standpoint.
There was a problem hiding this comment.
is there a way to make the path bar slide up and down in a transition? the movement would look a lot smoother as right now its a bit offputting, especially when you rapidly change between different files/folder and the path bar disappears/reappears. maybe setting some unique logic for the path bar for when the tag bar is active since this appears to only a problem when they are both active.
| onClick: () => void; | ||
| } | ||
|
|
||
| const TagItem = ({ |
There was a problem hiding this comment.
this should be refactored to another file since this file is already huge
| `h-[64px]` | ||
| )} | ||
| > | ||
| <em className="text-sm tracking-wide">{t('tags_bulk_instructions')}</em> |
There was a problem hiding this comment.
the italics look a bit strange, maybe just normal text is fine. But on that note I think this should be a tool tip instead, since it makes the tag bar unnecessarily large. Maybe putting a tooltip on the right hand side of the bar, or somewhere else that makes sense.
| "tags_bulk_assigned": "Assigned tag \"{{tag_name}}\" to {{file_count}} $t(file, { \"count\": {{file_count}} }).", | ||
| "tags_bulk_failed_with_tag": "Could not assign tag \"{{tag_name}}\" to {{file_count}} $t(file, { \"count\": {{file_count}} }): {{error_message}}", | ||
| "tags_bulk_failed_without_tag": "Could not tag {{file_count}} $t(file, { \"count\": {{file_count}} }): {{error_message}}", | ||
| "tags_bulk_instructions": "Select one or more files and press a key to assign the corresponding tag.", |
There was a problem hiding this comment.
this wording is rather misleading because it only allows you to assign number keys 1-9 to tags, so it might be beneficial to specify that
There was a problem hiding this comment.
I didn't test with more than 9 tags though so I definitely may be wrong here!
There was a problem hiding this comment.
curious what does this part translates to: $t(file, { \"count\": {{file_count}} }). Looks weird to have it on i18n
There was a problem hiding this comment.
changes to the proper singular/plural form of "file"/"files" based on the file_count number. it's the reason I added the actual numerical translations for the "file" key
This PR introduces tag assign mode to Spacedrive.
Changes:
Closes #2350