Closed z-roth closed 2 years ago
The code looks good but I'll do a full code review when everything is more finalized. As far as design goes, I'd prefer:
Making it harder to create could be as simple as putting it in a modal.
Also you might wanna do away with dividing lines and background on each Risk. Different bootstrap component (or just a divs) might help with this.
Also if you wanna get fancier with UI, you make the right side buttons be icons instead, only show on hover, and maybe add tooltips to say their function after mouse hovers for 2 seconds. This is very justifiable because rn as buttons, you're calling more attention to them than they really deserve. The number of primary buttons on a page at a given time should be kept meaningfully minimal.
Changed around a few things:
Going to look into how to add some icons and tooltips.
@jamescd18 are there any pre-existing examples of the use of icons and tooltips that I can look at to implement those UI improvements?
@jamescd18 are there any pre-existing examples of the use of icons and tooltips that I can look at to implement those UI improvements?
There should be an example of tooltips on the implemented changes section of the change request detail page. Not sure about making the button an icon, I'd look up a react-bootstrap example or smthn
i'm good with this from a visual standpoint. I'll leave it to Reid for the nitty gritty of the code
This looks a lot cleaner! There might be some stuff we wanna change/ add to the checklist component later, but for the purpose of this ticket this is awesome. Gonna go ahead and approve.
lgtm !!
Changes
Notes
Test Cases
CheckList
RiskLog
Screenshots (if applicable)
Checklist
It can be helpful to check the
Checks
andFiles changed
tabs. Please review the contributor guide and reach out to your squad if anything is unclear. Please request reviewers and ping on slack only after you've gone through this whole checklist.package-lock.json
changes (unless dependencies have changed)Closes #696