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

Remove form generator completely #2628

Merged
merged 6 commits into from Jul 14, 2023
Merged

Remove form generator completely #2628

merged 6 commits into from Jul 14, 2023

Conversation

ciyer
Copy link
Contributor

@ciyer ciyer commented Jun 30, 2023

  • Remove form generator from dataset edit path
  • Remove form generator code

Fix #2622
Fix #2356
Fix #1930

/deploy #persist #cypress renku=renku-ui-3.10-tests extra-values=core.horizontalPodAutoscaling.minReplicas=1,core.replicaCount=1

@ciyer ciyer temporarily deployed to renku-ci-ui-2628 June 30, 2023 14:39 — with GitHub Actions Inactive
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-2628.dev.renku.ch

@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 7, 2023 07:37 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 7, 2023 13:03 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 7, 2023 15:14 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 7, 2023 15:56 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 8, 2023 08:56 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 10, 2023 08:25 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 10, 2023 11:09 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 11, 2023 07:13 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 11, 2023 09:02 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 11, 2023 09:52 — with GitHub Actions Inactive
@ciyer ciyer linked an issue Jul 13, 2023 that may be closed by this pull request
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Wow, quite some changes in this PR 🚀

The code looks great already! I found a few little issues -- some not necessarily related to changes you introduced but just in the code you moved around or touched:

  • The cards in the datasets project tab look a bit broken
    image

  • The same goes for some of the Cards inside the dataset details page
    image

  • (Not new to this PR, but worth fixing) the "Deleting datasets" message we show in the "Delete Dataset" modal floats to the right
    image

  • This is not UI related, but I couldn't fully test a few operations due to the renku-core problems. E.G. importing a dataset into a project never ended even though it was imported.
    Do you mind adding these extra values to the deploy string? It should also help with the tests extra-values=core.horizontalPodAutoscaling.minReplicas=1,core.replicaCount=1
    image

client/src/components/form-field/CreatorsInput.tsx Outdated Show resolved Hide resolved
client/src/components/form-field/CreatorsInput.tsx Outdated Show resolved Hide resolved
client/src/components/form-field/CreatorsInput.tsx Outdated Show resolved Hide resolved
client/src/components/form-field/CreatorsInput.tsx Outdated Show resolved Hide resolved
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 13, 2023 15:14 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 13, 2023 17:10 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2628 July 14, 2023 07:20 — with GitHub Actions Inactive
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

This looks ready!

One detail: does it actually fix #1930 ? I still see the folders expanded on dataset view and edit pages
image

@ciyer
Copy link
Contributor Author

ciyer commented Jul 14, 2023

Thanks for the review. All good points! I have addressed all your requests, I think. The dataset delete now looks like this:
image

@ciyer
Copy link
Contributor Author

ciyer commented Jul 14, 2023

One detail: does it actually fix #1930 ? I still see the folders expanded on dataset view and edit pages image

Yes, there is now a more intelligent logic to determine if the folder should be displayed open or closed. The top level is shown opened if there are less than 5 elements there. See https://renku-ci-ui-2628.dev.renku.ch/projects/sean/another-playground-project/datasets/using_open_citation_data/

image

@ciyer ciyer deployed to renku-ci-ui-2628 July 14, 2023 09:16 — with GitHub Actions Active
@lorenzo-cavazzi
Copy link
Member

Yes, there is now a more intelligent logic to determine if the folder should be displayed open or closed. The top level is shown opened if there are less than 5 elements there.

I see. It's a bit different than what the original issue suggested and I fear it might not work well in some scenarios (e.g. more deeply nested folder structures with fewer elements per folder) but let's give this a go

Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

🚀

@ciyer ciyer merged commit cbc5570 into master Jul 14, 2023
12 of 14 checks passed
@ciyer ciyer deleted the ciyer/2415-dataset-edit branch July 14, 2023 09:46
ciyer pushed a commit that referenced this pull request Jul 14, 2023
minor: improve appearance of dataset delete
ciyer pushed a commit that referenced this pull request Jul 14, 2023
@RenkuBot
Copy link
Contributor

Tearing down the temporary RenkuLab deplyoment for this PR.

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