Skip to content

Dash salesforce crm#166

Merged
celinehuang merged 79 commits intomasterfrom
dash-salesforce-crm
Aug 5, 2019
Merged

Dash salesforce crm#166
celinehuang merged 79 commits intomasterfrom
dash-salesforce-crm

Conversation

@ptr0x00
Copy link
Copy Markdown
Contributor

@ptr0x00 ptr0x00 commented Jun 21, 2019

Issue for app: #8

App pull request

  • This is a new app
  • I am improving an existing app (redesigns/code "makeovers")

About

Workflow

  • I have created a branch in the appropriate monorepo, and the
    elements necessary for successful deployment are in place.
  • If the app is a redesigned and/or restyled version of an
    existing gallery app, I've summarized the changes requested in the
    appropriate Streambed issue and confirm that they have been applied.
  • If the app is on the Dash Gallery portal, I have added a link to
    the GitHub repository for the source code in the portal description.
  • I have removed all Google Analytics code from the app's
    assets/ folder.

The pre-review review

I have addressed all of the following questions:

  • Does everything in my code serve some purpose? (I have removed
    any dead and/or irrelevant code.)
  • Does everything in my code have a clear purpose? (My code is
    readable and, where it isn't, it has been commented appropriately.)]
  • Am I reinventing the wheel? (I have used appropriate packages to
    lessen the volume of code that needs to be maintained.)

@ptr0x00 ptr0x00 requested a review from YunkeXiao June 21, 2019 15:55
@ptr0x00 ptr0x00 marked this pull request as ready for review June 25, 2019 16:16
@ptr0x00 ptr0x00 requested a review from shammamah-zz June 25, 2019 16:33
@shammamah-zz
Copy link
Copy Markdown
Contributor

@Mskycoder Looks like there's an application error on the playground deployment. Could you please fix that?

Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

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

I can't see the playground app -- can you please make sure that the link is correct?

Comment thread apps/dash-salesforce-crm/index.py Outdated

app.layout = html.Div(
[
# header
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can probably get rid of these comments!

@@ -0,0 +1,32 @@
certifi
Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz Jun 28, 2019

Choose a reason for hiding this comment

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

Please lock all of these versions, and make sure that they are all required (I don't think all of them should be). We want to keep this file as short as possible.

Comment thread apps/dash-salesforce-crm/Procfile Outdated
@@ -0,0 +1 @@
web: gunicorn index:server No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should also have the --pythonpath part (see the README of this repository for an example of a Procfile).

@celinehuang celinehuang self-assigned this Aug 1, 2019
@celinehuang celinehuang requested review from chriskfwoo and removed request for YunkeXiao August 1, 2019 19:23
Comment thread apps/dash-salesforce-crm/README.md Outdated

```

To run the app, please create a SalesForce developer account (link is in the `About the App` section)and put the credentials in the `secrets.example.sh` file.
Copy link
Copy Markdown
Contributor

@chriskfwoo chriskfwoo Aug 1, 2019

Choose a reason for hiding this comment

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

They will have to copy the file and rename it secrets.sh and run the script to export the variables. I think it's worth mentioning it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why would they have to rename the file to secrets.sh? The command is source secrets.example.sh to provide the login info right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

secrets.example.sh is meant to be like a template file. If they modify that file, they might push the credentials to github. However, if they modify the secrets.sh file, it won't matter since it's .gitignore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense.

Comment thread apps/dash-salesforce-crm/requirements.txt Outdated
Comment thread apps/dash-salesforce-crm/app.py Outdated
Comment thread apps/dash-salesforce-crm/index.py Outdated
Comment thread apps/dash-salesforce-crm/index.py Outdated
Comment thread apps/dash-salesforce-crm/requirements.txt Outdated
Comment thread apps/dash-salesforce-crm/sfManager.py Outdated
Comment thread apps/dash-salesforce-crm/secrets.example.sh
@celinehuang celinehuang requested a review from chriskfwoo August 2, 2019 17:58
chriskfwoo
chriskfwoo previously approved these changes Aug 5, 2019
@shammamah-zz
Copy link
Copy Markdown
Contributor

@celinehuang Could you please, in the mobile layout:

  1. Make each of these dropdowns half of the width of the screen, and
  2. Make the button full-width?

Screen Shot 2019-08-05 at 2 19 55 PM

Other than that, looks good to me :)

@celinehuang
Copy link
Copy Markdown
Contributor

celinehuang commented Aug 5, 2019

Make the button full-width?

the mock up has it like this
image should I make it full width still? @shammamah

@shammamah-zz
Copy link
Copy Markdown
Contributor

No need to make it full width, but please center the dropdowns!

Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

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

Nice work 💃

@celinehuang celinehuang merged commit fdf543c into master Aug 5, 2019
@josegonzalez josegonzalez deleted the dash-salesforce-crm branch December 23, 2019 18:24
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.

6 participants