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.

Sign up for GitHub

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

Jump to bottom

Allow to add preparations in loan form #3659

Merged
merged 3 commits into from
Jul 3, 2023
Merged

Allow to add preparations in loan form #3659

merged 3 commits into from
Jul 3, 2023

Conversation

CarolineDenis
Copy link
Contributor

Fixes #2135

@CarolineDenis CarolineDenis mentioned this pull request Jun 22, 2023
Closed
maxpatiiuk
maxpatiiuk requested changes Jun 24, 2023
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

All good, just small thing

specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx Outdated Show resolved Hide resolved
maxpatiiuk
maxpatiiuk approved these changes Jun 30, 2023
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

Looks good.
The index -1 issue is something you could fix here or open a separate issue for.
Also, could compare to how it worked before - if we are not making it worse than before, then it's fine to leave it for the future

@CarolineDenis CarolineDenis requested a review from a team June 30, 2023 14:26
@CarolineDenis
Copy link
Contributor Author

For tests:

part1.mov
part2.mov

Notes: it takes a few seconds to see the new infos for the new added record (you might see the infos from record 1 for a few seconds before the updates)

bronwyncombs
bronwyncombs approved these changes Jul 3, 2023
View reviewed changes
Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

Works! Added preparations to loan from record set and by entering catalog numbers. Added from subview and grid view.

msedge_SiRgfew5RP.mp4

@CarolineDenis CarolineDenis merged commit 4ad26fd into xml-editor Jul 3, 2023
8 of 9 checks passed
@CarolineDenis CarolineDenis deleted the issue-2135-b branch July 3, 2023 14:49
@CarolineDenis CarolineDenis mentioned this pull request Feb 14, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxpatiiuk maxpatiiuk maxpatiiuk approved these changes

@bronwyncombs bronwyncombs bronwyncombs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
General Tester Board
Status: ✅Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@CarolineDenis @maxpatiiuk @bronwyncombs