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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creator tools header #204

Merged
merged 9 commits into from
Jan 26, 2018
Merged

Creator tools header #204

merged 9 commits into from
Jan 26, 2018

Conversation

eoji
Copy link
Contributor

@eoji eoji commented Jan 22, 2018

what

New creator tools header designs.

馃憖

@eoji
Copy link
Contributor Author

eoji commented Jan 22, 2018

@dnywh
Copy link
Contributor

dnywh commented Jan 22, 2018

https://github.com/kickstarter/kickstarter/pull/9474

@dnywh
Copy link
Contributor

dnywh commented Jan 22, 2018

Should we set some time aside this sprint for visual polish (getting the spacing and type sizing right), or set it aside for a later, general, UI sweep?

@eoji
Copy link
Contributor Author

eoji commented Jan 22, 2018

@dannyalright Thanks for the strings PR! And I think it should be this sprint or when we're getting ready to release this feature!

@eoji eoji mentioned this pull request Jan 23, 2018
Copy link
Contributor Author

@eoji eoji left a comment

Choose a reason for hiding this comment

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

Paired with @cdolm92 and made recommendations. Will update!

@@ -60,6 +69,21 @@ protected void onCreate(final @Nullable Bundle savedInstanceState) {
.compose(bindToLifecycle())
.compose(observeForUI())
.subscribe(this::newProjectClicked);

final BottomSheetBehavior.BottomSheetCallback bottomSheetCallback = new BottomSheetBehavior.BottomSheetCallback() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make this a private method for readability.

@@ -18,6 +20,7 @@
void projectSwitcherProjectClicked();
}
interface Outputs {
Observable<DateTime> projectLaunchDate();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you test this? 馃

@@ -34,16 +34,16 @@
}

interface Outputs {
/* project and associated stats object */
/** project and associated stats object */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For consistency, let's make these proper sentences.

@@ -5,11 +5,9 @@
xmlns:tools="http://schemas.android.com/tools"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see if we can move the new toolbar design here.

@@ -1,176 +1,220 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
android:id="@+id/creator_dashboard_funding_view"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See if we can extract styles from the new funding rows.

android:layout_width="match_parent"
android:layout_height="1dp"
android:background="@color/ksr_dark_grey_500"
android:orientation="vertical" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lil new line

-javadoc
-tests
-cleanup
@eoji
Copy link
Contributor Author

eoji commented Jan 25, 2018

Bumping @cdolm92! I went through and addressed all our feedback from yesterday.

Copy link

@cdolm92 cdolm92 left a comment

Choose a reason for hiding this comment

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

looks g2g after what we've discussed. Thanks for the android intro! 馃槃

@eoji eoji merged commit 6480367 into master Jan 26, 2018
@eoji eoji deleted the creator-tools-header branch January 26, 2018 15:31
Rcureton pushed a commit that referenced this pull request Jul 31, 2018
* Creator tools header.

* Cleanup and test updates.

* updated strings and moving style to v-21

* lint

* using proper "project" string

* PR feedback
-javadoc
-tests
-cleanup
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.

None yet

3 participants