-
Notifications
You must be signed in to change notification settings - Fork 83
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-4631 (1): Implement sensor addition UI flow #3659
base: integration
Are you sure you want to change the base?
LF-4631 (1): Implement sensor addition UI flow #3659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good -- I love the little mock 'loading' 😂
Regarding the linting of the routes file -- I don't think I was the last one to format those lines but yes, my formatter is in agreement with integration (i.e. it will try to revert the changes on this branch).
It looks like my prettier config is in alignment with prettierrc.json, but I also don't think any of these properties can create the diff here anyway! 🤔 My prettier extension version is 11.0.0.
{
"parser": "babel",
"printWidth": 100,
"trailingComma": "all",
"tabWidth": 2,
"semi": true,
"singleQuote": true
}
We might have to do some sleuthing in tech daily. FWIW I think prefer how the arrow functions are arranged here with whatever your settings are!
@@ -533,6 +541,7 @@ const Routes = ({ isCompactSideMenu, isFeedbackSurveyOpen, setFeedbackSurveyOpen | |||
<Route path="/create_location/fence" exact component={PostFenceForm} /> | |||
<Route path="/create_location/buffer_zone" exact component={PostBufferZoneForm} /> | |||
<Route path="/create_location/watercourse" exact component={PostWatercourseForm} /> | |||
<Route path="/create_location/sensor" exact component={PostSensorForm} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this route should also be in the other admin code block (manager / extension officer)... unless was this described as something only the farm owner should be permitted to do?
@@ -0,0 +1,80 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really my sense that sensors would ever (even when we expand the functionallity) become a real location in the previous way again, so I think I would be in favour of both files and routes centering around e.g. farm_addon
(to match the backend).
But I guess this depends on how we think expansion is going to work (whether it will be like reviving the old flow) so I'd like to check this with Anto! Also I don't think the decision has to be made yet.
padding: 0; | ||
min-width: 24px; | ||
min-height: 24px; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe background-color: transparent
would be a better default for this one? Just in case the parent container doesn't get a white background? There was a bit of that white-offwhite clash in Storybook which I'm probably being too picky about... 😝
Description
Rough implementation of the sensor addition flow. The goal is to set up the basic structure and functionality to prevent a large PR later.
ContextForm
:SIMPLE_HEADER
variant (story), which might not be used.PostSensor
container (/create_location/sensor
)NOTE:
ContextForm
for better scalability, expecting that we may eventually support sensors from other manufacturers or custom sensors.Jira link: https://lite-farm.atlassian.net/browse/LF-4631
New ticket: https://lite-farm.atlassian.net/browse/LF-4691
Type of change
How Has This Been Tested?
Checklist: