Skip to content

Circuit detail view [part 1] & fixes on list view #241

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

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

loris-olivier-obi
Copy link
Contributor

@loris-olivier-obi loris-olivier-obi commented Apr 3, 2025

Detail view

  • Header: action (download)
  • Header: Metadata + circuit data
  • Hero image
  • Overview: images of graphs for the moment (future dynamic usage)
  • Provenance: publications list
  • Related publications: publication list
  • Related circuit: parent circuit, subcircuits, deriverd from circuit (with download)

@g-bar
Copy link
Contributor

g-bar commented Apr 3, 2025

fixes on list view

What does it fix?

@loris-olivier-obi
Copy link
Contributor Author

fixes on list view

What does it fix?

I turned all the into to reach the detail view.
I haven't resolved yet the radio button unselect feature. That's the next step.

@g-bar
Copy link
Contributor

g-bar commented Apr 3, 2025

I turned all the into to reach the detail view.
I haven't resolved yet the radio button unselect feature. That's the next step.

Plesae ensure downloads don't break, that was a pain.

@loris-olivier-obi
Copy link
Contributor Author

I turned all the into to reach the detail view.
I haven't resolved yet the radio button unselect feature. That's the next step.

Plesae ensure downloads don't break, that was a pain.

I won't touch the logic you've done. By the way I use it everywhere else where circuits are used.
I keep trying to keep the native radio button from AntD table, and in case I don't find a solution today I will just add a column with a custom radio button and will hook it to select the row without touching to what you did.

Copy link
Contributor

@g-bar g-bar left a comment

Choose a reason for hiding this comment

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

You should update the circuit counts on main page and menu to use the map.

Object.keys(map).length

Copy link
Collaborator

@tolokoban tolokoban left a comment

Choose a reason for hiding this comment

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

That's a lot of work here. Well done!

I have added few comments.
But, I think you forgot to launch npm run lint before pushing.
If the linter fails locally, it's better to fix the code before the next push.

Copy link
Collaborator

@tolokoban tolokoban left a comment

Choose a reason for hiding this comment

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

There is just a little comment, but all the rest has been fixed, except for the linter that still complains on my machine.

Copy link
Collaborator

@pgetta pgetta left a comment

Choose a reason for hiding this comment

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

@loris-olivier-obi, that is terrific. Nice work.

I've left some comments, please see yourself what's relevant.

bilalesi
bilalesi previously approved these changes Apr 25, 2025
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.

6 participants