Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix navigation tree not showing up #278

Merged
merged 2 commits into from
May 12, 2020

Conversation

loan-laux
Copy link
Contributor

Signed-off-by: Loan Laux loan@outgrow.io

Resolves #256
Impact: critical
Type: bugfix

Issue

The Navigation page wasn't working because of the navigation tree not showing up.

Solution

Update react-dnd and react-dnd-html5-backend to 9.5.1, and move DndProvider up the component tree (at <App /> level) to avoid conflict with react-router.

Breaking changes

None.

Testing

  1. Visit the Navigation page.
  2. See the navigation tree.
  3. Try moving stuff around. It should work.

Signed-off-by: Loan Laux <loan@outgrow.io>
@loan-laux
Copy link
Contributor Author

I've noticed that there's no way to edit a newly added component item before clicking the Save button. Was that the case before or is this a regression?

@loan-laux
Copy link
Contributor Author

@willopez I can't assign a reviewer here for some reason... Since you were planning on fixing this, do you want to do a code review?

@willopez
Copy link
Member

Thanks for this @loan-laux! I'll review it, and I believe that was the case and this PR does not cause a regression. I'll verify when reviewing.

@mikemurray
Copy link
Member

I agree with @willopez, I don’t think your changes caused a regression.

Signed-off-by: Loan Laux <loan@outgrow.io>
Copy link
Member

@mikemurray mikemurray left a comment

Choose a reason for hiding this comment

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

👍 Look good.

Had to run tests locally since they didn't run on CI. This was fixed on CircleCI and they should run on future pull-requests from forked repos.

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

LGTM

@willopez willopez merged commit 3f460a3 into reactioncommerce:trunk May 12, 2020
@loan-laux
Copy link
Contributor Author

Thanks @willopez @mikemurray. Can we get a release for that? There's also a bunch of other things that are on trunk but not on beta.6, so it would make sense to release a beta.7 IMO.

@loan-laux loan-laux deleted the outgrow-fix-navigation branch May 13, 2020 08:44
@willopez
Copy link
Member

@loan-laux You are welcomed and thank you for you contribution. We'll get a new beta out soon.

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.

Can't see nor edit navigation tree
3 participants