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

Implement drag and drop for area climbs #840

Closed
wants to merge 1 commit into from
Closed

Conversation

zichongkao
Copy link
Contributor

@zichongkao zichongkao commented May 23, 2023

We made subareas drag-and-droppable in #696.

This PR achieves the same for climbs in an area. The main work is to abstract out a new DraggableTable component which we then re-use for climbs and subareas.

Screen Recording 2023-05-22 at 7 16 50 PM

@vercel
Copy link

vercel bot commented May 23, 2023

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

Name Status Preview Updated (UTC)
open-tacos ✅ Ready (Inspect) Visit Preview May 23, 2023 1:17am

@zichongkao zichongkao temporarily deployed to Preview May 23, 2023 01:15 — with GitHub Actions Inactive
@zichongkao zichongkao requested a review from vnugent May 23, 2023 01:19
rowIds={Array.from(areaStore.keys())}
editMode={editMode}
onDragEnd={onDragEnd}
renderRow={(areaId: string, idx: number) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A little bit inelegant, but I don't really know of a better design pattern. We need to pass in both the child component and the data to fill it with. One other idea is to pass in a list of childProps.

<DraggableTable ..>
  <AreaItem><AreaItem ...>
<DraggableTable ..>

But then we would have to maintain extra state here in the parent component to order the child components, which is worse than slightly ugly code IMO.

@vnugent
Copy link
Contributor

vnugent commented May 23, 2023

I haven't looked at this closely. Quick question, does content in the CSV editor stay in sync?

@zichongkao
Copy link
Contributor Author

I haven't looked at this closely. Quick question, does content in the CSV editor stay in sync?

Good call out -- it doesn't sync. I'll need to fix that.

@vnugent
Copy link
Contributor

vnugent commented May 23, 2023

I haven't looked at this closely. Quick question, does content in the CSV editor stay in sync?

Good call out -- it doesn't sync. I'll need to fix that.

if it's too complicated, I'd prioritize fixing other bugs since there's already an alternate way to order climbs using the bulk editor.

@vnugent
Copy link
Contributor

vnugent commented May 26, 2023

We let react-hook-form manage the form state. The CSV editor manipulates the climb list here:

const { replace, fields } = useFieldArray({ name, rules: gradeHelper.getBulkValidationRules() })

Let's postpone this improvement. I think the climb edit page can use some UX improvement

@vnugent
Copy link
Contributor

vnugent commented Mar 14, 2024

Obsolete.

@vnugent vnugent closed this Mar 14, 2024
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.

2 participants