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

Feature: Table redesign #1300

Merged
merged 7 commits into from
Oct 23, 2024
Merged

Feature: Table redesign #1300

merged 7 commits into from
Oct 23, 2024

Conversation

AkashJana18
Copy link
Contributor

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Changed hover effect, added search bar in 2 pages and added button as per requirement.

  • Why was this change needed? (You can also link to an open issue here)
    Fixes: 🚀 Feature: Table redesign #1286

  • Other information:
    image
    image

Copy link

vercel bot commented Oct 11, 2024

@AkashJana18 is attempting to deploy a commit to the Arc53 Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@ManishMadan2882 ManishMadan2882 left a comment

Choose a reason for hiding this comment

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

Hi @AkashJana18,
Just reviewed, have few points:

  • Make sure the UI matches the figma
  • The Add new button triggers the Upload modal
  • Search bar filters documents depending on document name
    Also try to sync with main as there some tab design related changes in this file from your previous PR.
    Thanks!

@AkashJana18
Copy link
Contributor Author

Hey @ManishMadan2882 the required changes are made can you please check it out.
Feel free to ask for more changes if required :)

Copy link
Collaborator

@ManishMadan2882 ManishMadan2882 left a comment

Choose a reason for hiding this comment

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

Hi @AkashJana18
Have few points:

  1. There is an app crash on creating a new API Key
  2. When the modal opens on clicking the New Document button, it partially covers the screen
    image
  3. For table re-design, please follow figma, but you can ignore pagination for now as it will need few endpoints.
  4. Also, please sync your branch with upstream main, there are conflicts.
    Thanks!

@dartpain
Copy link
Contributor

Hey @AkashJana18 how is it going?

@AkashJana18
Copy link
Contributor Author

@dartpain working on it PR will updated soon

@AkashJana18
Copy link
Contributor Author

Hey @ManishMadan2882 I have made the required changes

  1. Can you specify the kind of app crash on creating new key?
  2. Also can guide me regarding resolving conflict. Like do i need to keep the skeleton reloader or remove it?

@ManishMadan2882
Copy link
Collaborator

Hi @AkashJana18

  1. The app crashes on creating new api key
    The error is logged in the console
    Uncaught TypeError: Cannot read properties of undefined (reading 'toLowerCase') at APIKeys.tsx:93:20
  2. We need to keep every change in the main branch including the skeleton.
    Thanks.

@AkashJana18
Copy link
Contributor Author

Hey @ManishMadan2882 conflicts have been resolved.
Can you please check if the apikey error is resolved or not.
Thank you.

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.45%. Comparing base (67a97a7) to head (d18cb37).
Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1300   +/-   ##
=======================================
  Coverage   33.45%   33.45%           
=======================================
  Files          64       64           
  Lines        3273     3273           
=======================================
  Hits         1095     1095           
  Misses       2178     2178           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ManishMadan2882
Copy link
Collaborator

Hey @AkashJana18
Do you mind testing these changes before pushing or is there any issue you are facing during set up on your system?

Fails with npm run dev

[plugin:vite:react-babel] /home/manish/Documents/contributors/akash jana/DocsGPT/frontend/src/settings/APIKeys.tsx: Unexpected token, expected "," (57:6)
  60 |       });
/home/manish/Documents/contributors/akash jana/DocsGPT/frontend/src/settings/APIKeys.tsx:57:6
55 |            );
56 |          }
57 |        })
   |        ^
58 |        .catch((error) => {
59 |          console.error(error);

Copy link

vercel bot commented Oct 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-gpt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 9:59pm

@AkashJana18
Copy link
Contributor Author

@ManishMadan2882 how do i revert the changes made while resolving conflict. I think i made error while resolving conflict if you can revert the change in APIkeys.tsx i can fix it.
i resolved conflict from github itself (not from vscode).

@ManishMadan2882
Copy link
Collaborator

ManishMadan2882 commented Oct 23, 2024

First revert your merge commit, then Follow this on VSC

Copy link
Collaborator

@ManishMadan2882 ManishMadan2882 left a comment

Choose a reason for hiding this comment

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

Perfect, Thanks @AkashJana18

@dartpain dartpain merged commit 13c890b into arc53:main Oct 23, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Feature: Table redesign
3 participants