UnlockedLabs / UnlockEdv2

UnlockedLabs' WIP education portal for capturing meaningful progress of incarcerated learners in external providers, to help earn good time credits
5 stars 17 forks source link

Refactor HeroIcons into an Icon component supporting Tooltips #383

Open jtucholski opened 1 week ago

jtucholski commented 1 week ago

When revamping the Resources Management page, many icons were added with duplicate code. This code, including Tailwind CSS styles and DaisyUI tooltip components, could be encapsulated within something like a ULIcon component (or something generically named). Doing so would ensure consistency over time and reduce redundant code throughout the application.

Example of redundant code.

<div className="tooltip" data-tip="Delete Collection">
  <TrashIcon
       className="w-4 h-4 cursor-pointer"
       onClick={() =>onDeleteCollectionClick(collection.id) }
   />
</div>

It would be nice instead to have an icon that gives us the choice to add components like

<ULIcon 
    tooltip="Delete Collection" 
    className="w-4 h-4 cursor-pointer" 
    onClick={() =>onDeleteCollectionClick(collection.id) }
    icon={TrashIcon}
/>

This only applies to stand-alone icons not ones inside of button elements. An example of stand alone icons are the "Action" icons in the Users page, shown in the image below:

Screenshot 2024-09-24 at 1 10 38 PM

Icons that WILL NOT use this component are ones that are within button elements, like the "+ Add Provider" button shown below:

Screenshot 2024-09-24 at 1 09 44 PM
calisio commented 1 week ago

The second format is from an old way of doing things- so I think it definitely is a good idea to standardize this. They should all look like the first component you specified.. the icon should act as the button (we shouldn't be making a button with the icon in it.. I left a comment on the PR actually pointing this out). I think this is from the first code I wrote on the project and I was trying to replicate a design that was given to me, but since then I have solely been using icons as the buttons themselves. Also, all icons should have a w-4 on them. Maybe this can be included in the component so that way we don't have to retype that and we make sure all icons are the same size (and if needed can be overwritten inline).

jtucholski commented 1 week ago

@calisio Default classes added to the icon should probably include: w-4 h-4 cursor-pointer. Anything else stick out?

calisio commented 4 days ago

I think that should be it