Merge pull request #3035 from sumitdaga/issue-2177

fixes issue 2177

Author: maxceem

src/components/ScrollToAnchors.jsx

@@ -43,7 +43,7 @@ export function scrollToAnchors(Component) {
     componentDidMount() {
       const { hash } = window.location
 
-      if (hash !== '') {
+      if (!this.props.disableAutoScrolling && hash !== '') {
         // Push onto callback queue so it runs after the DOM is updated,
         // this is required when navigating from a different page so that
         // the element is rendered on the page before trying to getElementById.

src/projects/actions/projectDashboard.js

@@ -1,15 +1,9 @@
 import _ from 'lodash'
 import { loadMembers } from '../../actions/members'
-import { loadProject, loadProjectInvite, loadDirectProjectData, loadProjectPhasesWithProducts } from './project'
+import { loadProject, loadProjectInvite, loadDirectProjectData } from './project'
+import { loadProjectPlan } from './projectPlan'
 import { loadProjectsMetadata } from '../../actions/templates'
-import { loadProductTimelineWithMilestones } from './productsTimelines'
-import { loadFeedsForPhases } from './phasesTopics'
-import { LOAD_PROJECT_DASHBOARD,
-  LOAD_ADDITIONAL_PROJECT_DATA,
-  DISCOURSE_BOT_USERID,
-  CODER_BOT_USERID,
-  TC_SYSTEM_USERID
-} from '../../config/constants'
+import { LOAD_PROJECT_DASHBOARD, LOAD_ADDITIONAL_PROJECT_DATA } from '../../config/constants'
 
 /**
  * Load all project data to paint the dashboard
@@ -44,28 +38,7 @@ const getDashboardData = (dispatch, getState, projectId, isOnlyLoadProjectInfo)
       // for new projects load phases, products, project template and product templates
       if (project.version === 'v3') {
         promises.push(
-          dispatch(loadProjectPhasesWithProducts(projectId))
-            .then(({ value: phases }) => {
-              loadFeedsForPhases(projectId, phases, dispatch)
-                .then((phaseFeeds) => {
-                  let phaseUserIds = []
-                  _.forEach(phaseFeeds, phaseFeed => {
-                    phaseUserIds = _.union(phaseUserIds, _.map(phaseFeed.topics, 'userId'))
-                    _.forEach(phaseFeed.topics, topic => {
-                      phaseUserIds = _.union(phaseUserIds, _.map(topic.posts, 'userId'))
-                    })
-                    // this is to remove any nulls from the list (dev had some bad data)
-                    _.remove(phaseUserIds, i => !i || [DISCOURSE_BOT_USERID, CODER_BOT_USERID, TC_SYSTEM_USERID].indexOf(i) > -1)
-                  })
-                  // take difference of userIds identified from project members
-                  phaseUserIds = _.difference(phaseUserIds, userIds)
-
-                  dispatch(loadMembers(phaseUserIds))
-                })
-              // load timelines for phase products here together with all dashboard data
-              // as we need to know timeline data not only inside timeline container
-              loadTimelinesForPhasesProducts(phases, dispatch)
-            })
+          dispatch(loadProjectPlan(projectId, userIds))
         )
       }
 
@@ -96,26 +69,6 @@ const getData = (dispatch, getState, projectId, isOnlyLoadProjectInfo) => {
     .catch(() => getDashboardData(dispatch, getState, projectId, isOnlyLoadProjectInfo))
 }
 
-/**
- * Load timelines for phase's products
- *
- * @param {Array}   phases    list of phases
- * @param {Function} dispatch dispatch function
- */
-function loadTimelinesForPhasesProducts(phases, dispatch) {
-  const products = []
-
-  phases.forEach((phase) => {
-    phase.products.forEach((product) => {
-      products.push(product)
-    })
-  })
-
-  return Promise.all(
-    products.map((product) => dispatch(loadProductTimelineWithMilestones(product.id)))
-  )
-}
-
 export function loadProjectDashboard(projectId, isOnlyLoadProjectInfo = false) {
   return (dispatch, getState) => {
     return dispatch({

src/projects/actions/projectPlan.js

@@ -0,0 +1,58 @@
+import _ from 'lodash'
+import { loadMembers } from '../../actions/members'
+import { loadProjectPhasesWithProducts } from './project'
+import { loadFeedsForPhases } from './phasesTopics'
+import { loadProductTimelineWithMilestones } from './productsTimelines'
+import {
+  DISCOURSE_BOT_USERID,
+  CODER_BOT_USERID,
+  TC_SYSTEM_USERID
+} from '../../config/constants'
+
+
+/**
+ * Load timelines for phase's products
+ *
+ * @param {Array}   phases    list of phases
+ * @param {Function} dispatch dispatch function
+ */
+function loadTimelinesForPhasesProducts(phases, dispatch) {
+  const products = []
+
+  phases.forEach((phase) => {
+    phase.products.forEach((product) => {
+      products.push(product)
+    })
+  })
+
+  return Promise.all(
+    products.map((product) => dispatch(loadProductTimelineWithMilestones(product.id)))
+  )
+}
+
+export function loadProjectPlan(projectId, existingUserIds) {
+  return (dispatch) => {
+    return dispatch(loadProjectPhasesWithProducts(projectId))
+      .then(({ value: phases }) => {
+        loadFeedsForPhases(projectId, phases, dispatch)
+          .then((phaseFeeds) => {
+            let phaseUserIds = []
+            _.forEach(phaseFeeds, phaseFeed => {
+              phaseUserIds = _.union(phaseUserIds, _.map(phaseFeed.topics, 'userId'))
+              _.forEach(phaseFeed.topics, topic => {
+                phaseUserIds = _.union(phaseUserIds, _.map(topic.posts, 'userId'))
+              })
+              // this is to remove any nulls from the list (dev had some bad data)
+              _.remove(phaseUserIds, i => !i || [DISCOURSE_BOT_USERID, CODER_BOT_USERID, TC_SYSTEM_USERID].indexOf(i) > -1)
+            })
+            // take difference of existingUserIds identified from project members
+            phaseUserIds = _.difference(phaseUserIds, existingUserIds)
+
+            dispatch(loadMembers(phaseUserIds))
+          })
+        // load timelines for phase products here together with all dashboard data
+        // as we need to know timeline data not only inside timeline container
+        loadTimelinesForPhasesProducts(phases, dispatch)
+      })
+  }
+}

src/projects/detail/ProjectDetail.jsx

@@ -64,9 +64,6 @@ const ProjectDetailView = (props) => {
   if (!currentMemberRole && props.currentUserRoles && props.currentUserRoles.length > 0) {
     currentMemberRole = props.currentUserRoles[0]
   }
-  const regex = /#(feed-([0-9]+)|comment-([0-9]+))/
-  const match = props.location.hash.match(regex)
-  const ids = match ? { feedId: match[2], commentId: match[3] } : {}
 
   const { component: Component } = props
   const componentProps = {
@@ -79,7 +76,7 @@ const ProjectDetailView = (props) => {
     isProcessing: props.isProcessing,
     allProductTemplates: props.allProductTemplates,
     productsTimelines: props.productsTimelines,
-    ...ids
+    location: props.location,
   }
   return <Component {...componentProps} />
 }

src/projects/detail/components/EditProjectForm/EditProjectForm.jsx

@@ -268,7 +268,7 @@ class EditProjectForm extends Component {
 
 
   render() {
-    const { isEdittable, showHidden, productTemplates, productCategories } = this.props
+    const { isEdittable, showHidden, productTemplates, productCategories, disableAutoScrolling } = this.props
     const { template } = this.state
     const { project, dirtyProject } = this.state
     const onLeaveMessage = this.onLeave() || ''
@@ -287,6 +287,7 @@ class EditProjectForm extends Component {
             sectionNumber={idx + 1}
             resetFeatures={this.onFeaturesSaveAttachedClick}
             showFeaturesDialog={this.showFeaturesDialog}
+            disableAutoScrolling={disableAutoScrolling}
             // TODO we shoudl not update the props (section is coming from props)
             validate={(isInvalid) => section.isInvalid = isInvalid}
             showHidden={showHidden}
@@ -353,7 +354,8 @@ class EditProjectForm extends Component {
 }
 
 EditProjectForm.defaultProps = {
-  shouldUpdateTemplate: false
+  shouldUpdateTemplate: false,
+  disableAutoScrolling: false,
 }
 
 EditProjectForm.propTypes = {
@@ -371,6 +373,7 @@ EditProjectForm.propTypes = {
   updateAttachment: PropTypes.func.isRequired,
   removeAttachment: PropTypes.func.isRequired,
   shouldUpdateTemplate: PropTypes.bool,
+  disableAutoScrolling: PropTypes.bool,
 }
 
 export default EditProjectForm

src/projects/detail/components/PhaseFeed/PhaseFeed.jsx

@@ -5,26 +5,49 @@
  */
 import React from 'react'
 import _ from 'lodash'
+import { withRouter } from 'react-router-dom'
 
 import ScrollableFeed from '../../../../components/Feed/ScrollableFeed'
 import spinnerWhileLoading from '../../../../components/LoadingSpinner'
+import { scrollToHash } from '../../../../components/ScrollToAnchors'
 
 import './PhaseFeed.scss'
 
-const PhaseFeedView = (props) => (
-  <div styleName="container">
-    <ScrollableFeed
-      {...{
-        ..._.omit(props, 'feed'),
-        ...props.feed,
-        id: (props.feed ? props.feed.id.toString() : '0'),
-        commentId: props.commentId
-      }}
-    />
-  </div>
-)
+class PhaseFeedView extends React.Component {
+  constructor(props) {
+    super(props)
+  }
+
+  componentDidMount() {
+    !_.isEmpty(this.props.location.hash) && this.handleUrlHash(this.props)
+  }
+
+  // when the phase feed is actually loaded/rendered scroll to the appropriate post depending on url hash
+  handleUrlHash(props) {
+    const hashParts = _.split(location.hash.substring(1), '-')
+    const phaseId = hashParts[0] === 'phase' ? parseInt(hashParts[1], 10) : null
+    if (phaseId === props.phaseId) {
+      setTimeout(() => scrollToHash(props.location.hash), 100)
+    }
+  }
+
+  render() {
+    return (
+      <div styleName="container">
+        <ScrollableFeed
+          {...{
+            ..._.omit(this.props, 'feed'),
+            ...this.props.feed,
+            id: (this.props.feed ? this.props.feed.id.toString() : '0'),
+            commentId: this.props.commentId
+          }}
+        />
+      </div>
+    )
+  }
+}
 
 const enhance = spinnerWhileLoading(props => !props.isLoading)
-const EnhancedPhaseFeedView = enhance(PhaseFeedView)
+const EnhancedPhaseFeedView = enhance(withRouter(PhaseFeedView))
 
 export default EnhancedPhaseFeedView

src/projects/detail/components/ProjectStage.jsx

@@ -5,6 +5,7 @@ import React from 'react'
 import PT from 'prop-types'
 import _ from 'lodash'
 import uncontrollable from 'uncontrollable'
+import { withRouter } from 'react-router-dom'
 
 import { formatNumberWithCommas } from '../../../helpers/format'
 import { getPhaseActualData } from '../../../helpers/projectHelper'
@@ -22,7 +23,6 @@ import ProductTimelineContainer from '../containers/ProductTimelineContainer'
 import NotificationsReader from '../../../components/NotificationsReader'
 import { phaseFeedHOC } from '../containers/PhaseFeedHOC'
 import spinnerWhileLoading from '../../../components/LoadingSpinner'
-import { scrollToHash } from '../../../components/ScrollToAnchors'
 
 const enhance = spinnerWhileLoading(props => !props.processing)
 const EnhancedEditProjectForm = enhance(EditProjectForm)
@@ -130,23 +130,28 @@ class ProjectStage extends React.Component{
     expandProjectPhase(phase.id, tab)
   }
 
-  componentDidUpdate() {
-    const { phaseState } = this.props
-    if (_.get(phaseState, 'isExpanded')) {
-      const scrollTo = window.location.hash ? window.location.hash.substring(1) : null
-      if (scrollTo) {
-        scrollToHash(scrollTo)
-      }
-    }
+  componentDidMount() {
+    !_.isEmpty(this.props.location.hash) && this.handleUrlHash(this.props)
   }
 
-  componentWillReceiveProps(nextProps) {
-    const { feedId, commentId, phase, phaseState, expandProjectPhase } = this.props
-    const { feed } = nextProps
-    if (!_.get(phaseState, 'isExpanded') && feed && (feed.id === parseInt(feedId) || feed.postIds.includes(parseInt(commentId)))){
-      expandProjectPhase(phase.id, 'posts')
+  componentDidUpdate(prevProps) {
+    const { location } = this.props
+    if (!_.isEmpty(location.hash) && location.hash !== prevProps.location.hash) {
+      this.handleUrlHash(this.props)
     }
+  }
+
+  // expand a phase if necessary depending on the url hash
+  handleUrlHash(props) {
+    const { expandProjectPhase, phase, location } = props
 
+    const hashParts = _.split(location.hash.substring(1), '-')
+    const phaseId = hashParts[0] === 'phase' ? parseInt(hashParts[1], 10) : null
+
+    if (phaseId && phase.id === phaseId) {
+      const tab = hashParts[2]
+      expandProjectPhase(phaseId, tab)
+    }
   }
 
   render() {
@@ -169,6 +174,7 @@ class ProjectStage extends React.Component{
       collapseProjectPhase,
       expandProjectPhase,
       commentAnchorPrefix,
+      isLoading,
 
       // comes from phaseFeedHOC
       currentUser,
@@ -253,6 +259,8 @@ class ProjectStage extends React.Component{
               projectMembers={projectMembers}
               onSaveMessage={onSaveMessage}
               commentAnchorPrefix={commentAnchorPrefix}
+              phaseId={phase.id}
+              isLoading={isLoading}
             />
           )}
 
@@ -278,6 +286,7 @@ class ProjectStage extends React.Component{
                 removeAttachment={this.removeProductAttachment}
                 attachmentsStorePath={attachmentsStorePath}
                 canManageAttachments={!!currentMemberRole}
+                disableAutoScrolling
               />
             </div>
           }
@@ -311,7 +320,7 @@ ProjectStage.propTypes = {
   commentAnchorPrefix: PT.string,
 }
 
-const ProjectStageUncontrollable = uncontrollable(ProjectStage, {
+const ProjectStageUncontrollable = uncontrollable(withRouter(ProjectStage), {
   activeTab: 'onTabClick',
 })
 

src/projects/detail/components/ProjectStages.jsx

@@ -10,6 +10,7 @@ import { withRouter } from 'react-router-dom'
 import { formatNumberWithCommas } from '../../../helpers/format'
 import { getPhaseActualData } from '../../../helpers/projectHelper'
 
+import spinnerWhileLoading from '../../../components/LoadingSpinner'
 import Section from '../components/Section'
 import ProjectStage from '../components/ProjectStage'
 import PhaseCardListHeader from '../components/PhaseCardListHeader'
@@ -93,13 +94,15 @@ const ProjectStages = ({
   collapseProjectPhase,
   feedId,
   commentId,
+  location
 }) => (
   <Section>
 
     <PhaseCardListHeader {...formatPhaseCardListHeaderProps(phases)}/>
     {
       phases.map((phase, index) => (
         <ProjectStage
+          location={location}
           key={phase.id}
           phaseState={phasesStates[phase.id]}
           productTemplates={productTemplates}
@@ -151,6 +154,10 @@ ProjectStages.propTypes = {
   updateProductAttachment: PT.func.isRequired,
   removeProductAttachment: PT.func.isRequired,
   deleteProjectPhase: PT.func.isRequired,
+  isLoadingPhases: PT.bool.isRequired,
 }
 
-export default withRouter(ProjectStages)
+const enhance = spinnerWhileLoading(props => !props.isLoadingPhases)
+const EnhancedProjectStages = enhance(ProjectStages)
+
+export default withRouter(EnhancedProjectStages)

src/projects/detail/components/SpecSection.jsx

@@ -425,6 +425,7 @@ SpecSection.propTypes = {
   updateAttachment: PropTypes.func,
   removeAttachment: PropTypes.func,
   productCategories: PropTypes.array.isRequired,
+  disableAutoScrolling: PropTypes.bool,
 }
 
 export default scrollToAnchors(SpecSection)