Leaf/a5.2 Delete Education #350
Conversation
blu3eee
left a comment
blu3eee
left a comment
There was a problem hiding this comment.
The changes look good to me. Nicely done! Just need to resolve the file conflicts before merging.
|
Also, I'm not sure how compatible this is with #341 as both PRs are working on the profile section. So it might be a good idea to communicate with tdang about his changes. Maybe working on merging 341 first, then you can make further changes as his work has been waiting for a while to be merged. Another idea I have is that you can create a new branch based on his branch and merge yours with it so both your work can be merged in one PR (in the case you cannot contact him on the matter) as his work now is also having same file names conflicts as yours. I apologize for the inconvenience caused by the refactoring. |
I brought this up last meeting about how the group accordion component was designed to be reused for both education and experience, so I think I'll be taking over his issue. It should be mostly the same as what I've done here for education with a few edits. |
resolves #28
I have implemented deletion for education. There should be a delete icon now for each education item, and you should be able to click on it to open a confirmation modal, and upon confirming the item should be deleted.
Further details
To implement this, I had to make changes to the group accordion component, which was previously designed to be reusable for the other groups for a user's profile as well. Adding and Editing was already implemented on the local side. However, the state data and its manipulation methods were stored within the group accordion, which would make it difficult to tailor a group accordion component and its actions to a specific field like education or interests, so I had to lift out the state and its manipulation methods out of the component so that it has to be passed into it instead. This should make it a lot easier to reuse this component for different profile fields.
Additionally the state manipulation methods for each accordion item was based on index, which I realized would also make it difficult for any future implementations for making changes on the backend so I added a property for an accordion id and made the state manipulation methods based on that.
You can see the state and its manipulation methods for education being passed down in
profile-page.tsxunder the component Profile Experience. I've defined three handlers for editing, deletion, and adding education, which are passed into the group accordion component and from there are further passed and tied to their corresponding components. I intended for these to include both backend and local manipulation in the future, but since this issue says not to handle anything backend related I just left a comment for those actions in the future.Note: The accordion combo boxes/textfield actually store their own state data of local editing changes being made to them, so all the edit handler is supposed to do is handle backend saving when they click the save icon after finishing their edits. Currently it does nothing but it is being called when you click on the save icons.
profile-accordion.tsx. I added a state boolean to control whether it's open or not, and tied deletion and modal dismissal to the yes button and modal dismissal to the no button.Sorry if all this is too out of scope for this issue, if its too much I can try separating these changes into different branches. I just figured that these changes were necessary for implementing deletion and beneficial for future implementations of working with backend, and that if I was going to implement deletion this way I might as well do it for adding/editing too.
What You Should See