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

refactor: react-table --> tanstack #72

Merged
merged 39 commits into from
Mar 7, 2024
Merged

refactor: react-table --> tanstack #72

merged 39 commits into from
Mar 7, 2024

Conversation

eldu
Copy link
Contributor

@eldu eldu commented Feb 27, 2024

  • Upgrades from react-table to tanstack
  • Removes URL, hyperlinks title instead
  • Mainly parity with previous version
  • Cleans up a lot of scss code that we don't need anymore

closes #51

@eldu eldu self-assigned this Feb 27, 2024
@eldu eldu changed the base branch from main to build-update February 27, 2024 22:22
Copy link

github-actions bot commented Feb 27, 2024

Visit the preview URL for this PR (updated for commit 2b9d6d3):

https://ccv-pubs--pr72-refactor-tanstack-8jxmokqb.web.app

(expires Mon, 11 Mar 2024 21:38:41 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f75c16b3bdd7d81b7a0b7f8d3eed826b541e7a2a

@eldu eldu changed the title Refactor tanstack refactor: react-table --> tanstack Feb 28, 2024
@eldu
Copy link
Contributor Author

eldu commented Mar 1, 2024

  • Center headers and cells vertically
  • Sort icon generic
  • Hyperlink title
  • Remove URL

@eldu eldu marked this pull request as ready for review March 1, 2024 23:52
@hetd54
Copy link

hetd54 commented Mar 5, 2024

A couple of data things you might not have control over, but I will flag:

Some, but not all author lists end in a comma:
Screenshot 2024-03-05 at 11 20 51 AM

Some title's have html tags within them:
Screenshot 2024-03-05 at 11 20 21 AM

Some pubs appear more than once:
Screenshot 2024-03-05 at 11 19 45 AM


.form-group.required > .form-label:after {
content: '*';
content: ' *';
Copy link

Choose a reason for hiding this comment

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

For my own understanding, what does adding a space do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if you open up the form modal, all the required fields have " *" next to it if it's a required field. The space in this case is just a space. So instead of DOI* we get DOI *

Screenshot 2024-03-05 at 4 42 23 PM

Copy link

@hetd54 hetd54 left a comment

Choose a reason for hiding this comment

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

Mobile issues

I cannot tap into the year filters on mobile.

Also on mobile, the chart's y axis labels get chopped off (also tested on my actual phone to make sure):
Screenshot 2024-03-05 at 11 27 08 AM

Otherwise looks great to me!

@eldu
Copy link
Contributor Author

eldu commented Mar 5, 2024

@hetd54

A couple of data things you might not have control over, but I will flag:

Some, but not all author lists end in a comma:

I added a comment on #71
There is a bunch of weird things with the data. Tim also pointed out #56

Mobile issues

I cannot tap into the year filters on mobile.

Also on mobile, the chart's y axis labels get chopped off (also tested on my actual phone to make sure):

Otherwise looks great to me!

I added a new issue #75 ! Thanks for pointing that out :)

@eldu eldu merged commit 12c4c8f into build-update Mar 7, 2024
2 checks passed
@eldu eldu deleted the refactor-tanstack branch March 7, 2024 15:50
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.

Upgrade react table to tanstack
2 participants