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

dash brain viewer #148

Merged
merged 26 commits into from Jul 8, 2019

Conversation

Projects
None yet
3 participants
@chriskfwoo
Copy link
Member

commented Jun 13, 2019

Issue for app: #24

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.
  • If the app is a reimplementation of a Python gallery app for the
    DashR gallery, the app in this PR mimics, as closely as possible,
    the style and functionality of the existing app.=
  • 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.)

@chriskfwoo chriskfwoo changed the title Dash brain viewer dash brain viewer Jun 13, 2019

@chriskfwoo chriskfwoo added the dash label Jun 13, 2019

@chriskfwoo chriskfwoo self-assigned this Jun 13, 2019

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Note

The button for the view on github will only work on master.

Style changes

Sidebar

  • made it fixed and scrollable if overflows
  • left-aligned subtitles

Mobile Layout

brain-mobile

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

I'm not sure if there's a better way to save custom annotations.

I have a function called save_annotations() that listens to the relayout_data and I able to get that edit
annotation text by getting the key scene.annotations[0].text in the relayout_data. I store the key and text value in the dcc store dictionnary.

Essentially, I have a dictionary that contains "scene.annotations[index_of_list].text" as keys. And, I have to store each text in the appropriate index in the figure list figure["layout"]["scene"]["annotations"][index]["text"].

The difficult part is that the path to the text is coded in the string in the key: "scene.annotations[index_of_list].text".

@chriskfwoo chriskfwoo requested review from shammamah and ronniebugia Jun 18, 2019

@chriskfwoo chriskfwoo marked this pull request as ready for review Jun 18, 2019

@ronniebugia
Copy link
Contributor

left a comment

The double scroll bar style on the right side looks kind of weird.

scroll bars

@shammamah
Copy link
Collaborator

left a comment

Screen Shot 2019-06-18 at 3 46 27 PM

This looks quite a bit darker than what's in the mockup -- did you use Avocode to get the color codes?
Show resolved Hide resolved apps/dash-brain-viewer/app.py
Show resolved Hide resolved apps/dash-brain-viewer/app.py Outdated
@ronniebugia
Copy link
Contributor

left a comment

Annotation text goes outside of the box. Maybe add a overflow scroll for the annotations?

annotation text

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2019

Screen Shot 2019-06-18 at 3 46 27 PM

This looks quite a bit darker than what's in the mockup -- did you use Avocode to get the color codes?

@shammamah Good call. See commit.

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2019

The double scroll bar style on the right side looks kind of weird.

scroll bars

This is a small edge case where the graph is too big (this will only happen on really low-resolution screens). I would think of a solution.

Annotation text goes outside of the box. Maybe add a overflow scroll for the annotations?

annotation text

This is the same behaviour currently on the gallery. The text only appears outside of the box only when you are editing. I don't think this is a problem.

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 21, 2019

@shammamah @ronniebugia please review my latest changes 😄

@shammamah

This comment has been minimized.

Copy link
Collaborator

commented Jun 21, 2019

@chriskfwoo Did you remove the ability to add annotations? I'm not able to do so any more

chriskfwoo added some commits Jun 26, 2019

@chriskfwoo

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

@chriskfwoo Did you remove the ability to add annotations? I'm not able to do so any more

@shammamah I added an issue about loading time for annotations. #173

Let me know if there's any other request changes.

@ronniebugia
Copy link
Contributor

left a comment

💃 Looks good

chriskfwoo added some commits Jun 28, 2019

@shammamah shammamah self-requested a review Jul 8, 2019

@shammamah
Copy link
Collaborator

left a comment

💃

@chriskfwoo chriskfwoo merged commit 766acec into master Jul 8, 2019

2 checks passed

ci/circleci: deploy_to_playground Your tests passed on CircleCI!
Details
ci/circleci: test_black Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.