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

LF-4378 Readonly/Edit animal details framework - add form framework for animal details #3489

Draft
wants to merge 8 commits into
base: integration
Choose a base branch
from

Conversation

kathyavini
Copy link
Collaborator

@kathyavini kathyavini commented Oct 4, 2024

Update Friday, Oct 4th:

  • Made the change discussed at the end of tech daily this morning with @Duncan-Brain and @SayakaOno (readonly/edit as component state instead of routes to avoid being able to go back to the /edit route)
    • I'm getting sick and I think my way of handling the modal used way too much state 😅 But I believe it has the behaviour we were interested in without interfering with the previous setup.
    • it's not used in this implementation nor is it related to the form implementation as far as I can see, but history.back() (regular browser back) doesn't happen to work from this route going back to inventory; did not investigate that yet
  • Also renamed the forms for clarity while waiting on decision about SingleStep variant vs SingleStep component
    • Please note the SingleStepWithReadOnlyEdit was still never cleaned up all the way...! It still has extra logic and that transition.block() behaviour should be refactored to a common hook; just haven't implemented yet

Okay, probably going to be out for the afternoon though now since this cold is making me feel awful!

Description

Still WIP, but wanted to get a POC for discussing the relationship between this form and MultiStepForm, as mentioned this morning in tech daily. This is more or less taking the parallelism between this form and AddAnimals to its greatest extent, by making readonly/edit a variant of <MultiStepForm />

  • Readonly and Edit have separate routes but one component (as was done in Finances... I like that pattern!)
  • The separate route means the transition.unblock can work as before (I haven't pulled out the common code yet but I would if we keep this pattern)
  • child components were left as placeholders indicating the tickets in which they would be completed
  • routing should be discussed as there were two different methods set up in AnimalRoutes and Inventory: create...Url() function and a path property. I ignored the path property and used create...Url() on this branch

Jira link: https://lite-farm.atlassian.net/browse/LF-4378

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@kathyavini kathyavini self-assigned this Oct 4, 2024
@kathyavini kathyavini requested review from a team as code owners October 4, 2024 00:56
@kathyavini kathyavini requested review from antsgar and SayakaOno and removed request for a team October 4, 2024 00:56
@kathyavini kathyavini marked this pull request as draft October 4, 2024 00:59
@kathyavini kathyavini added enhancement New feature or request draft labels Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant