Skip to content

Conversation

@sumitdaga
Copy link
Contributor

No description provided.

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sumitdaga. Fix seems to work good.

There is only one feature got broken as far I see - project deleting. See demo video https://monosnap.com/file/YZu12MSCJzaAeOKRg03JYIQGjOR1A5. While expected behaviour - move to project listing page after project deleting.

  • At the moment delete button is not shown, but we are going to show it back soon, so we should keep it working.

  • To show delete button you may adjust the code the next way during testing:

    image

  • The reason for this issue is how we check that project has been removed:

    image

I would suggest fixing it the next way:

  • inside src/projects/reducers/project.js create a reusable method getEmptyProjectObject which would return `{ invites: [], members: [] }.
  • reuse this method everywhere inside src/projects/reducers/project.js
  • export this method
  • use method getEmptyProjectObject inside src/projects/detail/ProjectDetail.jsx and update check for deleted project:
    • instead of _.isEmpty(project) check _.isEqual(getEmptyProjectObject(), project)

This way is far from perfect, though it looks like has the minimal change and we would be aware that empty project has a special structure, and in the future we wouldn't loose it.
If you have a better idea, it's highly welcomed.

@sumitdaga
Copy link
Contributor Author

@maxceem i have pushed as per your suggestion ...please let me know if its fine!

@maxceem maxceem self-requested a review April 23, 2020 12:54
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @sumitdaga works good.

@maxceem maxceem merged commit 15531cf into topcoder-archive:dev Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants