Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excel viewer displaying data #76

Open
wants to merge 3 commits into
base: new-api
Choose a base branch
from
Open

Conversation

ramses1998
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@SYoy SYoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General Feedback on dependecies

we want to limit our dependencies on external libraries, especially for very specific use. Therefore it would be great if you remove the "moment" library and write a js func instead.
Also remove the "swr" library and make use of "useMemo". Same for tailwind-merge, remove dependency on it.

If possible -> replace react-tabs for simple buttons since we do not use the concepts of tabs anywhere else.

Regarding the icons library "lucid-react", we can agree on switching from heroicons -> for this change, please identify where heroicons are used and also replace them to assert consistency. Then remove the "heroicons/react" lib

One personal request

The "New Conversation" button was introduced in the latest new-api merge on main. The button looks a bit weirdly placed in the ChatWindow component. It would be nice if you style it prettier when changing the Icons.

@SYoy
Copy link
Collaborator

SYoy commented Mar 3, 2025

I will revisit the code later on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants