Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

Autofill harvest year to current year #114

Conversation

badnames
Copy link
Member

@badnames badnames commented Mar 17, 2023

The aim of this commit is to prefill the year input field in the seed creation dialog with the current year (As requested in #35).
This is accomplished by adding a new defaultValue prop to SimpleFormInput.

Basics

  • Details of what you changed are in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, are in the commit messages.
  • The buildserver is happy.
  • The PR is rebased with current master.

Checklist

  • I added unit tests for my code
  • I fully described what my PR does in the documentation
    (not in the PR description)
  • I fixed all affected documentation
  • I fixed all affected decisions
  • I added code comments, logging, and assertions as appropriate
  • I mentioned every code not directly written by me in reuse syntax

Review

@badnames badnames requested a review from buenaflor March 17, 2023 07:10
@@ -1,6 +1,6 @@
import { FieldValues, Path, UseFormRegister } from 'react-hook-form';

import { HTMLInputTypeAttribute } from 'react';
import { useState, HTMLInputTypeAttribute } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

is useState needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, its not. I was initially going to use it, but I forgot to remove it in the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm surprised this goes through the pre-commit hook. We have a linter setup that should execute when committing, I'll have to take a look at it.

Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

left 1 comment but lgtm

@markus2330
Copy link
Contributor

Great to see the continuation of the work for the seed milestone. We are so much looking forward to it! 🚀

@buenaflor thank you so much for reviewing! ❤️

Is this rebased to current master? The deployment does seem not work? (According to Jenkins log and also https://pr.permaplant.net/seeds/new doesn't seem to have the auto-fill feature?)

@badnames
Copy link
Member Author

Is this rebased to current master? The deployment does seem not work? (According to Jenkins log and also
https://pr.permaplant.net/seeds/new doesn't seem to have the auto-fill feature?)

I think so....
GitHub shows a green tick mark, so I assumed that it is ready to merge.

@badnames badnames merged commit 3cb27ae into ElektraInitiative:master Mar 17, 2023
@badnames badnames deleted the autofill_harvest_year_to_current_year branch March 17, 2023 10:32
@markus2330
Copy link
Contributor

LGTM, on dev.permaplant.net it now shows the current year.

@badnames please don't self-merge unless there is an emergency etc. It is better if someone else merges. We also want two reviews per PR, so that everyone learns most parts of PermaplanT.

@buenaflor thank you for looking into it, please write your finding about linter in #98.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants