update project creation #5849

MaxBittker · 2017-08-09T18:27:49Z

This PR replaces project creation with a new react view that captures platform intent and re-uses components from the onboarding view. it also removes the old django view and moves the tests to jest and acceptance

https://media.giphy.com/media/xUPJUs84cSrQC2USVW/giphy.gif

ghost · 2017-08-09T18:29:55Z

	1 Warning
⚠️	You should update CHANGES due to the size of this PR

Generated by 🚫 danger

MaxBittker · 2017-08-23T18:55:34Z

interested on UX + look+feel feedback as well!

kjlundsgaard · 2017-08-23T20:32:26Z

This looks good! A couple things I noticed:

If you select a platform and go to the issue stream and then click installation instructions it takes you back to the platform picker where you need to resubmit the platform you're using.
During the configure your application section, you don't have access to the "star this project" or "project settings" buttons.

I guess its possible that 1 is desired behavior because maybe people will want to configure their project for another platform and 2 is desired because we don't want to distract people from the configuration process with other options. Not sure how likely people are to go through the flow and go back to configuration.

Overall it looks much better than the django view, though - I like it.

MaxBittker · 2017-08-23T21:34:26Z

I'm curious if it's safe to remove the old django view the way I did or if there's some complication i'm missing

billyvg · 2017-08-24T00:31:46Z

Looks great!

Some small/random thoughts:

(not a big deal here but this is the browser's autofill input, padding on input would make this look better)
What are the frameworks sorted by on the Popular tab? It seems like language name, framework name? Would it be better to do a sort by popularity?
- It might be weird only having Popular not sorted alphabetically - maybe have autofocus on the search input?
This is because I'm a superuser I'm assuming, but you can create a project on a team you are not a member of, and in the Stream view, the ProjectSelector component does not have a project selected. This might be ok if it only applies to superusers.
- If not then we should fix the Team dropdown, or style/sort so that it favors teams you are a member of.
"Continue" button doesn't really give you an idea of what's going to happen, vs something like "Create Project".
- I've noticed this on the old django view, and it's worse there (and worse in local dev env because of added latency), but you can mash the submit button and create multiple projects
Love the framework icon in Project name input!

denamwangi · 2017-08-25T00:24:28Z

This is such a big improvement Max. Most things have been noted but i'd say two small things that might be nice would be to widen the team picker dropdown to accommodate longer names since we have the space available.

I don't know how tricky it would be to take out but it would be nice to take the project picker dropdown out of this flow when creating a new project.

MaxBittker · 2017-08-25T00:59:20Z

i'lll widen it! katie also mentioned some header stuff but i want to leave that off for a next pass along with some of billy's comments

billyvg · 2017-08-28T18:40:29Z

src/sentry/static/sentry/app/views/onboarding/createProject.jsx

Let's use the OrganizationState mixin here

billyvg · 2017-08-28T18:40:46Z

src/sentry/static/sentry/app/views/onboarding/createProject.jsx

Doesn't react-router provide this as a prop?

https://github.com/getsentry/sentry/blob/new-project-flow/src/sentry/static/sentry/app/views/app.jsx#L28

you might be right - it's also used as context all over though ;/

if you grep for this.props.location it seems to be used more than this.context.location

billyvg · 2017-08-28T18:47:03Z

src/sentry/static/sentry/app/views/onboarding/createProject.jsx

is this necessary?

billyvg · 2017-08-28T18:48:25Z

src/sentry/static/sentry/app/views/onboarding/createProject.jsx

no change needed here, but thoughts on adding eslint rule for preferring shorthand?

initial impression is no but I could be convinced otherwise

https://eslint.org/docs/rules/object-shorthand

can be --fixed

cleaner

👍 I've given people this feedback repeatedly, if we can automate it, let's.

billyvg · 2017-08-28T18:51:15Z

src/sentry/static/sentry/app/views/onboarding/index.jsx

can we move this into render and is cloneElement needed here?

billyvg · 2017-08-28T18:52:20Z

src/sentry/static/sentry/app/views/onboarding/project/index.jsx

: is still inside of t()

you must have been eating your fish eyes

billyvg · 2017-08-28T18:57:02Z

src/sentry/static/sentry/app/views/onboarding/project/index.jsx

nit: I think it's a bit cleaner if we just invoke the method here (and more consistent w/ codebase)

:( this is cheekier. but you're right.

billyvg · 2017-08-28T18:57:39Z

src/sentry/static/sentry/app/views/projectInstall/newProject.jsx

OrganizationState

merge fix

fix percy fix acceptance tests

padding and cleanup move padding nvm about this formatting thing

billyvg · 2017-08-28T23:27:47Z

src/sentry/static/sentry/app/views/onboarding/createProject.jsx

+  },
+
+  contextTypes: {
+    organization: SentryTypes.Organization,


Ah by OrganizationState, I meant this mixin https://github.com/getsentry/sentry/blob/e2e221fa9cde5b00035ef8bc9386bde71bd721eb/src/sentry/static/sentry/app/mixins/organizationState.jsx

billyvg · 2017-08-28T23:36:07Z

src/sentry/static/sentry/app/views/projectInstall/newProject.jsx

+
+const newProject = React.createClass({
+  contextTypes: {
+    organization: SentryTypes.Organization


OrganizationState mixin

MaxBittker force-pushed the new-project-flow branch 3 times, most recently from 8dfacfb to 6d17794 Compare August 14, 2017 23:52

MaxBittker changed the title ~~New project flow [WIP]~~ New project flow Aug 16, 2017

MaxBittker changed the title ~~New project flow~~ update new project flow to use fancy onboarding components Aug 16, 2017

MaxBittker requested a review from denamwangi August 16, 2017 21:59

MaxBittker force-pushed the new-project-flow branch 3 times, most recently from 7cb0966 to f504e4d Compare August 18, 2017 21:34

MaxBittker changed the title ~~update new project flow to use fancy onboarding components~~ update project creation Aug 18, 2017

MaxBittker force-pushed the new-project-flow branch 2 times, most recently from 53e2288 to 57eddd0 Compare August 22, 2017 17:27

MaxBittker requested review from billyvg and kjlundsgaard August 23, 2017 18:53

MaxBittker force-pushed the new-project-flow branch from c32918a to 3191dfe Compare August 25, 2017 21:49

billyvg requested changes Aug 28, 2017

View reviewed changes

MaxBittker force-pushed the new-project-flow branch from 1e8ca44 to eb6605c Compare August 28, 2017 21:10

MaxBittker added 6 commits August 28, 2017 14:20

wip

917eae7

merge fix

wip refactor

da0dbeb

fix links

f6d07b0

todo: write tests for createProject

8ef565b

update tests

eaaff2b

remove title

2d9d353

MaxBittker added 4 commits August 28, 2017 14:20

protect from no members

c228483

team picker and styles

fcdbfdd

update snaps

90a059c

move acceptance test to frontend

5d3b698

MaxBittker force-pushed the new-project-flow branch 3 times, most recently from 2333efa to 6050f19 Compare August 28, 2017 21:31

MaxBittker added 11 commits August 28, 2017 14:31

fix acceptance tests

311d6c9

fix percy fix acceptance tests

try to fix percy

64e8bed

the user needs to be a member of the org

813e8a5

padding and cleanup move padding nvm about this formatting thing

acceptance tests

c8c33df

remove old route

63104fc

removed commented out code

de7c658

add explanation and prevent double-trigger

4286d4f

address review

63d3ab7

copy

ecc63d6

address billy review

92b2508

re-add yarn.lock

dd028d5

MaxBittker force-pushed the new-project-flow branch from 6050f19 to dd028d5 Compare August 28, 2017 21:31

billyvg requested changes Aug 28, 2017

View reviewed changes

billyvg self-requested a review August 28, 2017 23:56

billyvg approved these changes Aug 28, 2017

View reviewed changes

use organization-mixin

f84d49a

MaxBittker force-pushed the new-project-flow branch from db63849 to f84d49a Compare August 29, 2017 00:28

add render test for newProject

1e24247

MaxBittker merged commit d95c3eb into master Aug 29, 2017

mattrobenolt deleted the new-project-flow branch August 31, 2017 19:22

github-actions bot locked and limited conversation to collaborators Dec 22, 2020

Uh oh!

update project creation #5849

update project creation #5849

Uh oh!

Conversation

MaxBittker commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Aug 9, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxBittker commented Aug 23, 2017

Uh oh!

kjlundsgaard commented Aug 23, 2017

Uh oh!

MaxBittker commented Aug 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

billyvg commented Aug 24, 2017

Uh oh!

denamwangi commented Aug 25, 2017

Uh oh!

MaxBittker commented Aug 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MaxBittker commented Aug 9, 2017 •

edited

Loading

ghost commented Aug 9, 2017 •

edited by ghost

Loading

MaxBittker commented Aug 23, 2017 •

edited

Loading