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

feat(UI & API): Manage views in the frontend #414

Merged
merged 28 commits into from
Oct 3, 2024
Merged

Conversation

rouk1
Copy link
Contributor

@rouk1 rouk1 commented Sep 30, 2024

This PR allows user to create/duplicate/edit & delete views.

Backend

All project endpoints are now below the same prefix.

  • GET /api/project/items lists all items of a project.
  • POST /api/project/views/share/{key} returns a static shareable HTML file named {view name}-{yyyy-MM-dd-HH-mm}.html.
  • PUT /api/project/views/{key} save a view in the project.
  • DELETE /api/project/views/{key} delete a view from the project.

Frontend

  • A new EditableList component as been implemented to manage views.
  • A lot of renaming to get rid of the concept of "report".
  • Plug everything to the new layout.
  • Rework view card layout and add an action dropdown to it.
  • Rework dropdowns using floating UI to make them fixed.
manage-views.mp4

Co-authored with @augustebaum.
Fixes #336, #407, #377, #332

@rouk1 rouk1 linked an issue Sep 30, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Sep 30, 2024

Coverage Report for ./frontend

Status Category Percentage Covered / Total
🔵 Lines 79.87% 1461 / 1829
🔵 Statements 79.87% 1461 / 1829
🔵 Functions 44.89% 44 / 98
🔵 Branches 83.87% 130 / 155
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
frontend/src/ShareApp.vue 0% 0% 0% 0% 1-2, 4-5, 7, 11-18
frontend/src/main.ts 0% 0% 0% 0% 1, 3-4, 6-7, 9, 11-12, 14
frontend/src/models.ts 100% 100% 100% 100%
frontend/src/router.ts 100% 100% 0% 100%
frontend/src/share.ts 0% 0% 0% 0% 1-2, 4-5, 7-8, 10-13, 15-19
frontend/src/components/DataFrameWidget.vue 87.2% 85.71% 33.33% 87.2% 50-54, 56-59, 62, 103
frontend/src/components/DropdownButton.vue 100% 100% 100% 100%
frontend/src/components/DropdownButtonItem.vue 100% 100% 100% 100%
frontend/src/components/EditableList.vue 72.72% 100% 0% 72.72% 31-33, 35-37
frontend/src/components/EditableListItem.vue 100% 100% 100% 100%
frontend/src/components/SectionHeader.vue 100% 50% 0% 100%
frontend/src/components/SimpleButton.vue 100% 100% 100% 100%
frontend/src/services/api.ts 80.28% 57.14% 100% 80.28% 11-12, 22-23, 49-52, 61-62, 77-80
frontend/src/views/ComponentsView.vue 83.84% 100% 0% 83.84% 32-34, 36-38, 44-53, 55-59, 61-65, 67-69, 111-118, 120-140
Generated in workflow #18 for commit 7f96f45 by the Vitest Coverage Report Action

@rouk1 rouk1 changed the title 336 UI manage views feat(UI & API): manage views Oct 1, 2024
@rouk1 rouk1 changed the title feat(UI & API): manage views feat(UI & API): Manage views Oct 1, 2024
@thomass-dev thomass-dev self-requested a review October 1, 2024 08:04
@rouk1 rouk1 marked this pull request as ready for review October 2, 2024 14:23
@augustebaum
Copy link
Contributor

Really awesome!

@augustebaum
Copy link
Contributor

On the design side, it looks like removing a card from the view could be 1 click instead of 2? What do you think @anasstarfaoui ?

frontend/src/services/api.ts Outdated Show resolved Hide resolved
frontend/src/views/ProjectView.vue Outdated Show resolved Hide resolved
@augustebaum augustebaum changed the title feat(UI & API): Manage views feat(UI & API): Manage views in the frontend Oct 2, 2024
@anasstarfaoui
Copy link

That's an amazing app so far guys, big kuddos to all of you :)

Just one suggestion, would it be possible to fixed the position of the Views list by default? I feel like it would be preferrable to not have something who grows @rouk1 :)

@rouk1
Copy link
Contributor Author

rouk1 commented Oct 2, 2024

Just one suggestion, would it be possible to fixed the position of the Views list by default?

Could you elaborate please I don't understand your comment ? The view list is on top of the page and has a height of 20% of the height of the viewport.

Oh ! Understood it's minimum height is 2 times the inner padding it's maximum height is 20% of the height of the viewport then a scrollbar appears. Is it ok for you @anasstarfaoui ?

@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 2, 2024

Huge work 👏🏻 , the video is awesome.
It's a bit hard for me to evaluate anything other than python, as the diff is a bit long 😄 .
Will do my best if needed.

Copy link
Contributor

@augustebaum augustebaum left a comment

Choose a reason for hiding this comment

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

I found a UX issue: when first starting the UI, adding cards to the canvas doesn't work, with no explanation why. I think it's because no view is selected by default.
Some ways to solve this:

  • Select the first view in the list at startup
  • Allow the user to set a "default" view, and select that one at startup
  • Warn the user that they need to select a view

The easiest is probably the warning. @anasstarfaoui do you see a better way of dealing with this?

EDIT: Resolved

Copy link
Contributor

@augustebaum augustebaum left a comment

Choose a reason for hiding this comment

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

I think the "intermediate view" mechanic when creating a view is a bit strange. I would expect a view either to exist, or not to exist. Some alternatives we discussed with @rouk1:

  • If the user clicks away during the view naming process, either validate or cancel the view creation (like in VSCode when you create a file: if you click away and the text box is empty, the creation is aborted; if the text box is not empty, the file is created). Essentially, clicking away should be as effectful as pressing "Enter"
  • Ask for the view name before creating anything (e.g. in a modal)

EDIT: Resolved (if the user clicks away, the name is validated; also we now forbid more than 1 view to be in "pending" mode, and to name 2 views the same)

@rouk1
Copy link
Contributor Author

rouk1 commented Oct 3, 2024

I found a UX issue: when first starting the UI, adding cards to the canvas doesn't work, with no explanation why. I think it's because no view is selected by default. Some ways to solve this:

* Select the first view in the list at startup

* Allow the user to set a "default" view, and select that one at startup

* Warn the user that they need to select a view

The easiest is probably the warning. @anasstarfaoui do you see a better way of dealing with this?

@augustebaum an error toast will now pop if a user tries to double click or drag an item if no view is selected.

@thomass-dev thomass-dev merged commit 274b6f0 into main Oct 3, 2024
11 checks passed
@thomass-dev thomass-dev deleted the 336-ui-manage-views branch October 3, 2024 15:05
thomass-dev pushed a commit that referenced this pull request Dec 2, 2024
## This PR allows user to create/duplicate/edit & delete views.

### Backend

All project endpoints are now below the same prefix.

- `GET /api/project/items` lists all items of a project.
- `POST /api/project/views/share/{key}` returns a static shareable HTML
file named `{view name}-{yyyy-MM-dd-HH-mm}.html`.
- `PUT /api/project/views/{key}` save a view in the project.
- `DELETE /api/project/views/{key}` delete a view from the project.

### Frontend

- A new `EditableList` component as been implemented to manage views. 
- A lot of renaming to get rid of the concept of "report".
- Plug everything to the new layout.
- Rework view card layout and add an action dropdown to it.
- Rework dropdowns using floating UI to make them fixed.


https://github.com/user-attachments/assets/3f94e483-ecea-4d5a-8433-50f9150fc6e6


Co-authored with @augustebaum.
Fixes #336, #407, #377, #332

---------

Co-authored-by: Auguste Baum <auguste@probabl.ai>
Co-authored-by: Auguste Baum <52001167+augustebaum@users.noreply.github.com>
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.

[UI] Manage views
5 participants