web-app icon indicating copy to clipboard operation
web-app copied to clipboard
verified publisher icon openMF

feat: add tree view for offices component

Open rish106 opened this issue 6 years ago • 13 comments

Description

Added Tree View in Offices Component and designed its UI.

Features which deviate from community app:

  • Added View Office to show on clicking office in the tree.
  • Added 'Close Button' in View Office to close the current view for office.

Related issues and discussion

#149

Screenshots, if any

tree_view_3 tree_view_2

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [x] If you have multiple commits please combine them into one commit by squashing them.

  • [x] Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.

rish106 avatar Jan 21 '20 19:01 rish106

Great work!! I have reviewed your code, I just want to run it on my machine and then I will approve the task, I will do it by tonight as I am currently in Office. I know the deadline of submission is tonight but don't worry about it. The deadline for Mentor to review is till tomorrow.

Also, similar to Chart Account were some information is shown for the account when it is clicked. Could you check the possibility of adding the same for an Office? This would be an additional enhancement which you can work on separately (not as a part of the task).

I will try to do that, how much information about the office needs to be displayed?

rish106 avatar Jan 23 '20 09:01 rish106

We can try something similar to what is displayed while viewing an office (if possible)

abhaychawla avatar Jan 23 '20 09:01 abhaychawla

We can try something similar to what is displayed while viewing an office (if possible)

Screenshot_5 Is this layout fine? I've added getDatatables method in Organization Service so it can be used in the future for other components too. I'll push the changes once I get a green light from you. I also added a 'Close' button to close the current view of office.

rish106 avatar Jan 23 '20 12:01 rish106

Could I recommend you align expand all and collapse all to the left, or shift it so it is in line with the trees card when the information card appears.

punwai avatar Jan 23 '20 14:01 punwai

Apart from this, layout looks very good

punwai avatar Jan 23 '20 14:01 punwai

tree_view_2 Moved it all to the left for now.

rish106 avatar Jan 23 '20 14:01 rish106

Also, it is nothing major, but try to ensure the shadows and everything are consistent in the component. I can see that the left tree has a much stronger shadow and a sharp corner. If you decide to use a card for the information, it would be better to do the same for the tree.

punwai avatar Jan 23 '20 20:01 punwai

Also, it is nothing major, but try to ensure the shadows and everything are consistent in the component. I can see that the left tree has a much stronger shadow and a sharp corner. If you decide to use a card for the information, it would be better to do the same for the tree.

Keeping the mat-tree in a card doesn't affect its shadows so I don't think it is possible to do that.

rish106 avatar Jan 24 '20 16:01 rish106

@Rishabh106 Please drop all the commits except yours, to bring it to the initial state. Then you can follow the steps as mentioned here. To avoid any loss of code keep your code safe in another branch locally or keep a copied folder of the code.

muskankhedia avatar May 14 '20 13:05 muskankhedia

Sorry, I messed up as there were about 35 commits which had to be pulled. I'll drop them and then resolve the merge conflicts.

rish106 avatar May 14 '20 13:05 rish106

@Rishabh106 Can you resolve MCs please?

Yes, I've resolved them. Please inform me if I messed up something while resolving the MCs, haven't been active lately so there's room for error.

rish106 avatar Aug 25 '20 14:08 rish106

@karantakalkar is this PR now ready to be merged?

abhaychawla avatar Nov 04 '20 20:11 abhaychawla

@Rishabh106 @karantakalkar @abhaychawla @muskankhedia is this PR ready to be merged ??

bharathcgowda avatar Mar 30 '21 09:03 bharathcgowda

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

github-actions[bot] avatar Nov 09 '22 01:11 github-actions[bot]

@Rishabh106 @karantakalkar @abhaychawla @muskankhedia is this PR ready to be merged ??

should I resolve the merge conflicts?

Edit: just found out that the tree view has been added already by this commit , closing the PR now.

rish106 avatar Dec 04 '22 15:12 rish106