Closed FyreByrd closed 2 days ago
FyreByrd
commented
2 days ago Looks good to me for the most part. I'm curious the reasoning behind moving components into the lib dir. Is there a clear advantage to having them there instead of in the dir they will be used?
They are used in both /projects and /directory, so lib was the best place to put them
FyreByrd
commented
2 days ago Merging #1036 caused conflicts, so had to rebase and force-push this
FyreByrd
commented
2 days ago Fixed the button layout in the UI to be more mobile friendly. No major changes to how it looks non-mobile, so just including the mobile screenshots.
Before:
After:
Before:
After:
One of the components,
ProjectSelectorwas used in two separate locations (projects and project directory) with moderately different use cases. The differences between these use cases led to some ofProjectSelector's implementation being quite janky. It also relied on information returned from the server, which I don't think is a great design choice (unless that data is returned by the root+layout.server.ts, as is the case with the user session and the organization list), as there is no clear way to indicate that the component relies on having certain data being returned from the server.As such, I split the main UI portion into a
ProjectCardcomponent, which renders the information for a single project, and then duplicated the rest of theProjectSelectorcode into the two pages the component was originally used in and made the necessary modifications in each location to have only the code that was needed.Yes, there is still a decent amount of overlap/duplication; however, that will mostly be resolved in feature/paginate-projects which moves all of the sorting and filtering boilerplate to the server-side.
Yes, I could have just kept this as part of feature/paginate-projects, but some of the changes here are significant enough that I anticipated that it would conflict with changes in one of my other branches where I am adding more functionality in the projects page (archival), and I decided to break this out separately so I could rebase that other branch onto this one and resolve those conflicts now, rather than wait for the conflicts to be even more complicated.