jtorreggiani
commented
1 month ago Welcome @cpualei! We are excited to have you contributing to the project! A couple of things to consider before we can get this fix merged in.
- It looks like staging is behind the main branch (I'm going to update staging so it is in sync with the main)
- Updating staging should reduce the diff between your branch and staging to just your changes
- This ternary statement is OK, but typically we try to avoid one-off ternary statements like this because it can make the code more brittle. As soon as we want to do something like this for another specialty it will start to become unwieldy
- We should add a field for the button text in Contentful so we don't need to add additional conditional logic to the component
- Adding a new field in Contentful will enable the LuskinOIC team in the future to change other buttons without writing code
Once get staging updated, we can push your code to staging to test things out. We can merge this in to resolve the immediate request.
We could prioritize moving this into a Contentful field. Since making updates in Contentful requires admin access you will need to work with our team to implement those additional changes. You can email me at jtorreggiani@specollective.org to schedule a time to connect.
YolineB
Pull Request Process
Copy and Paste / Write ticket description
made changes to button text for hand and microsurgery specialty card
Hand Trauma department requested for 'Patient' and 'Physician' buttons to read 'Trauma' and 'Congenital/Microsurgery' instead.
Describe the changes you've made to resolve the issue
Added a ternary which checks if the slug reads 'hand-trauma'. If so, change the button text to read 'Trauma'.
Added a ternary which checks if the slug reads 'hand-trauma'. If so, change the button text to read 'Trauma'. Made changes to the 'app/specialties/SpecialtyComponents/SpecialtyCard.tsx' file
Add screenshots of the UI before and after your changes have been made
Before
After
Code Review Checklist