openproject icon indicating copy to clipboard operation
openproject copied to clipboard

Feature/31918 Collapse/Expend all work packages in hierarchy view

Open Eric-Guo opened this issue 1 year ago • 7 comments

Largely copy from WorkPackageViewCollapsedGroupsService but not consider the view state save in query.

https://community.openproject.org/projects/openproject/work_packages/31918/activity

Eric-Guo avatar Jul 10 '24 08:07 Eric-Guo

eslint error not relative with this PR and passed in local in lefthook running eslint.

Eric-Guo avatar Jul 10 '24 13:07 Eric-Guo

First of all, thanks @Eric-Guo. This PR here is much welcomed especially since it addresses a much requested feature request.

I did not really look into the code, but at a glance, I think the following is necessary:

  • Adding feature specs for the functionality.
  • Double checking the UX by @marcalcobe or @psatyal. To that end, I added a screen capture of the functionality:

Kapture 2024-07-12 at 10 00 53

ulferts avatar Jul 12 '24 08:07 ulferts

@ulferts Thanks doing the screen record for me, I will try to add spec and do other necessary change to make this PR merged.

I known OpenProject team want to do a feature complete and elegant than just only can work, at the same time, @thape-cn current using OpenProject to build project management solution to meet our hard (if not hit in the bottom) real property/archtecture design market in China.

I really don't want to leave too much unique change in our branch, so really want to merge as much feature as possible. 😄

Eric-Guo avatar Jul 12 '24 08:07 Eric-Guo

Hello. Yes, thank you @Eric-Guo for this feature PR! This is indeed an of-requested functionality.

In addition to the Feature you link in the original PR message, there is also this Feature request with more detailed acceptance criteria, which we can use as a base: https://community.openproject.org/projects/openproject/work_packages/29852/activity

Based on what I'm seeing in the screen capture (thank you, @ulferts!), the implementation looks quite elegant. A few questions:

  • Which icons are you using for the button and the 'Collapse all' and 'Expand all' options? We'll need to convert them to Primer Octicon styles (we could do this on our end).
  • Is the collapse all/expand all setting saved in the query?
  • Is the collapse all/expand all setting saved across multiple pages?

Depending on your answers, I'd be in favour of merging the two Feature work packages, associating this PR with the merged Feature and then collaborating with the front-end team to also ensure that the implementation meets their requirements too. We can then go through our internal testing process on that Feature.

Thoughts, @ulferts?

Cheers Parimal

psatyal avatar Jul 12 '24 14:07 psatyal

@psatyal , I guess the PR does not implement everything that is requested in the feature's acceptance criteria. E.g. currently the setting is not saved in the view. I nevertheless think that it is one step into the right direction. So I would do the following:

  • Tailor one of the work packages to have the scope covered by the PR reflected for the purpose of the release notes (e.g. use https://community.openproject.org/wp/31918 for this)
  • Use the other work package for the next steps, removing everything from it then already covered (by this PR).
  • In case we still want to improve anything:
    • Hide the feature behind a feature flag. Ideally before merging but we can also do it on our end afterwards.
    • Improve the UX to the extend we think necessary.
    • Remove the feature flag.

That way, we can offer some improvements to our users, honor the work done by @Eric-Guo and do the rest when it seems appropriate.

ulferts avatar Jul 15 '24 09:07 ulferts

Hi @ulferts,

Sounds like a good approach to me. Let's do this.

@Eric-Guo, could you give us a functional overview in text of what this feature does? I'll update the existing work package with the relevant acceptance criteria and the other work package with the feature set that this PR does not cover.

Cheers Parimal

psatyal avatar Jul 15 '24 13:07 psatyal

@Eric-Guo , nothing new is probably following here but I would like to revive this which is why I recapitulate what is necessary from my point of view:

  • Feature specs done by you
  • Review done by a one of us maintainers
  • Writeup about the intended scope of this PR. Ideally done by you as well, @Eric-Guo . This would help with the next step.
  • Processing of the tickets to differentiate between what we have done in this PR and what is still open to be handled in the future.

ulferts avatar Aug 13 '24 08:08 ulferts