Update reactstrap and edit/create modal validation #958
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- upgrade to bootstrap v5 and reactstrap v9 - uses of react-bootstrap (modals and buttons) now use reactstrap - convert HeaderButtonsComponent to a NavBar - preliminary work with adding in-form feedback for validation
- Adding instant validation and feedback to the modal create/edit forms - some design discussions are needed before the final layout can be done
- Edit/Create modals now use 100% reactstrap components - Added defined autocomplete behavior - Changed the way labels are defined to comply with HTML standards - Added clear lists of what is validated for each modal - Rearrange the navbar header
Reactstrap's default color is "secondary", and many buttons relied on this. To make the expected behavior more clear, every button now has a color defined.
- Moved around some logic to make the component tree more linear - Simplified optionsVisibility state - Convert HeaderButtonsComponent and MenuModalComponent to use react hooks - Show navbar on all non-graph pages even when optionsVisibility is false
|
Thanks to @spearec for taking this on. Overall, this looks good. I have not yet reviewed the code but did try using it. Here are some thoughts:
Once you feel this is ready then formally submit and it will have all the code reviewed. |
- Increased modal width to match - Modals will switch back to one column at the lg breakpoint - Non-admin group view is always one column
- Component hasn't been used in ~5 years - Also fix formatting for EditMeterModalComponent
Closed
|
With the latest commits, this PR should be ready. I have no done much testing yet, so I would not deem it ready to merge until it gets tested more. |
- fix default language not being set properly - fix a couple jsdoc comments - add validation for groups to not allow submission without child meters
This was referenced Jul 13, 2023
Closed
- some tooltips and text entries were removed by this pr but still in data.js
huss
reviewed
Jul 20, 2023
There was a problem hiding this comment.
Thanks to @spearec for more very good work. It has taken too long to get this review done so I'm posting my comments from reading the code. I still need to carefully test it running. There are a few thoughts but nothing significant.
src/client/app/components/conversion/CreateConversionModalComponent.tsx
Outdated
Show resolved
Hide resolved
src/client/app/components/conversion/EditConversionModalComponent.tsx
Outdated
Show resolved
Hide resolved
huss
reviewed
Jul 24, 2023
- move help URL to a single exported constant - fix uses of double quotes - require conversion slope and intercept inputs to always have values
- Various help tooltips move to be inline - Fixed OpenEnergyDashboard#907, preventing admins from setting meter units to be displayable
|
Thanks to @spearec for addressing all the comments and even fixing some other issues they found. Everything looks good. I changed the wording on one message but that was minor. This is ready to merge and will be included in v1.0.0. |
huss
approved these changes
Jul 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The main scope of this PR is to remove the uses of
react-bootstrapand replace them withreactstrap.The
react-bootstrappackage was used in the header buttons component and in edit/create modal components.In the process of updating these components, I changed their UI significantly. The header buttons are now a navbar, which supports multiple compact dropdowns and menus.
Edit/create modals now include in-form validation and are displayed in two columns on larger screens. This allows OED to give instant feedback to admins, as well as prevent them from trying to submit an invalid meter in most cases. In the process of adding validation, I also prevented meter units from being displayable. The two-column layout allows for more information to be displayed at once and will revert to the old one-column layout on small screens.
Also fixes the issue that the admin and local language preferences were the same, so an admin could not change their display language without potentially affecting the site default. In doing so, a new state section (state.options) and options dropdown in the navbar were created, which can be used to add similar user-specific settings in the future.
Fixes #903
Fixes #907
Partially Addresses #726 (Header & Language containers removed)
Type of change
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)