Skip to content

UI Updates to Tap Detail View #124

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.

Sign up for GitHub

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

Open
johnnyruz wants to merge 6 commits into
base: master
Choose a base branch
from
Open

UI Updates to Tap Detail View #124

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de64fab
Add ABV/IBU text to Tap Details
May 18, 2020
914e7cc
Show tap/beverage notes on the tap details page, with configuration o…
May 18, 2020
5cdd401
Make badge text smaller to improve fit on some tablets (tested on Ace…
May 18, 2020
e9282ec
(Issue #102) - Clean up layout on Pour In Progress to prevent Done b…
May 18, 2020
e62fc7b
First updates based on patfreeman suggestions
johnnyruz May 19, 2020
7a7b1e8
Reverse layout changes to pour in progress screen, did not look good …
May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions kegtab/src/main/java/org/kegbot/app/TapStatusFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ private void updateTapDetails() {

final TextView title = ButterKnife.findById(mView, R.id.tapTitle);
final TextView subtitle = ButterKnife.findById(mView, R.id.tapSubtitle);
final TextView abvText = ButterKnife.findById(mView, R.id.tapAbv);
final TextView ibuText = ButterKnife.findById(mView, R.id.tapIbu);
final TextView tapNotes = ButterKnife.findById(mView, R.id.tapNotes);
final ViewFlipper flipper = ButterKnife.findById(mView, R.id.tapStatusFlipper);

Expand Down Expand Up @@ -256,6 +258,21 @@ public void onClick(View v) {
description = tap.getDescription();
}

// Find ABV and IBU values
final String abv = String.valueOf(keg.getBeverage().getAbvPercent());
if(mCore.getConfiguration().getAbvVisibleWhenZero()) {
abvText.setText("ABV: " + abv + "%");

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd personally prefer it to read more naturally, like 6.7% ABV

@johnnyruz johnnyruz May 19, 2020 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I could agree with that, thoughts on also flipping the IBU display? I think having both labels as a prefix or suffix would look best vs. having one as 6.7% ABV and the other IBU: 50. EDIT: Missed the comment below!

} else{
abvText.setText(keg.getBeverage().getAbvPercent() == 0 ? "" : "ABV: " + abv + "%");

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should probably just set the visibility to GONE. abvText.setVisibility(View.GONE);

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, fixing

}

final String ibu = String.valueOf(Math.round(keg.getBeverage().getIbu()));
if(mCore.getConfiguration().getIbuVisibleWhenZero()){
ibuText.setText("IBU: " + ibu);

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd personally prefer it to read more naturally, like 17 IBUs

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

On it!

} else{
ibuText.setText(keg.getBeverage().getIbu() == 0 ? "" : "IBU: " + ibu);

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should probably just set the visibility to GONE. ibuText.setVisibility(View.GONE);

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, fixing

}

final ImageView tapImage = (ImageView) mView.findViewById(R.id.tapImage);
final ImageView tapIllustration = (ImageView) mView.findViewById(R.id.tapIllustration);

Expand Down Expand Up @@ -291,14 +308,15 @@ public void onClick(View v) {
final Image image = keg.getBeverage().getPicture();
final String imageUrl = image.getUrl();
mImageDownloader.download(imageUrl, tapImage);
} else if (!Strings.isNullOrEmpty(description)) {
tapImage.setVisibility(View.GONE);
}

showIllustration(!keg.getBeverage().hasPicture());

if (!Strings.isNullOrEmpty(description) && mCore.getConfiguration().getDisplayTapNotes()) {
tapNotes.setVisibility(View.VISIBLE);
tapNotes.setText(description);
}

showIllustration(true);

// TODO(mikey): proper units support
// Badge 1: Pints Poured
final BadgeView badge1 = (BadgeView) mView.findViewById(R.id.tapStatsBadge1);
Expand Down
12 changes: 12 additions & 0 deletions kegtab/src/main/java/org/kegbot/app/config/AppConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,18 @@ public boolean getTemperaturesCelsius() {
return getBoolean(ConfigKey.TEMPERATURE_UNITS_CELSIUS);
}

public boolean getAbvVisibleWhenZero() {
return getBoolean(ConfigKey.ABV_DISPLAY_WHEN_ZERO);
}

public boolean getIbuVisibleWhenZero() {
return getBoolean(ConfigKey.IBU_DISPLAY_WHEN_ZERO);
}

public boolean getDisplayTapNotes() {
return getBoolean(ConfigKey.DISPLAY_TAP_NOTES);
}

public boolean stayAwake() {
return getBoolean(ConfigKey.STAY_AWAKE);
}
Expand Down
5 changes: 5 additions & 0 deletions kegtab/src/main/java/org/kegbot/app/config/ConfigKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ enum ConfigKey {
VOLUME_UNITS_METRIC(FALSE),
TEMPERATURE_UNITS_CELSIUS(FALSE),

ABV_DISPLAY_WHEN_ZERO(FALSE),
IBU_DISPLAY_WHEN_ZERO(FALSE),

DISPLAY_TAP_NOTES(TRUE),

STAY_AWAKE(TRUE),
KEEP_SCREEN_ON(TRUE),
WAKE_DURING_POUR(FALSE),
Expand Down
14 changes: 6 additions & 8 deletions kegtab/src/main/res/layout/pour_in_progress_activity.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

<!-- Right box: Camera Preview & Controls -->

<LinearLayout
<RelativeLayout

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This change hide the Done Pouring button.

@johnnyruz johnnyruz May 22, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Aligned camera view to bottom but reverted changes to Done Button/Image/Shout text to maintain visibility on smaller tablet screens.

android:id="@+id/pourInProgressRightCol"
android:layout_width="0dip"
android:layout_height="match_parent"
Expand Down Expand Up @@ -94,7 +94,7 @@
<org.kegbot.app.util.SoftMultiLineEditText
android:id="@+id/shoutText"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_height="128dip"
android:layout_alignParentTop="true"
android:layout_toRightOf="@+id/pourDrinkerImage"
android:hint="@string/pour_shout_hint"
Expand All @@ -107,24 +107,22 @@
<Button
android:id="@+id/pourEndButton"
style="@style/mediumButton"
android:layout_below="@+id/shoutText"
android:layout_toRightOf="@+id/pourDrinkerImage"
android:layout_alignBottom="@+id/pourDrinkerImage"
android:layout_below="@+id/pourDrinkerImage"
android:text="@string/pour_button_done"/>
</RelativeLayout>

</LinearLayout>

</ViewFlipper>

</ViewFlipper>

<fragment
android:id="@+id/camera"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_alignParentBottom="true"
class="org.kegbot.app.camera.CameraFragment"
tools:layout="@layout/camera_fragment_layout">
</fragment>
</LinearLayout>
</RelativeLayout>

</LinearLayout>
12 changes: 12 additions & 0 deletions kegtab/src/main/res/layout/tap_detail.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@
android:layout_toRightOf="@+id/tapTitle"
android:layout_alignBaseline="@+id/tapTitle"/>

<TextView
style="@style/beverageDetails"
android:id="@+id/tapAbv"
android:layout_alignParentRight="true"/>

<TextView
style="@style/beverageDetails"
android:id="@+id/tapIbu"
android:layout_alignBaseline="@id/tapTitle"
android:layout_alignParentRight="true"
android:layout_below="@id/tapAbv"/>

<View
android:id="@+id/tap_detail_divider"
android:layout_width="match_parent"
Expand Down
35 changes: 17 additions & 18 deletions kegtab/src/main/res/layout/tap_detail_loaded.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,25 @@
</org.kegbot.app.view.BadgeView>
</LinearLayout>

<TextView
android:id="@+id/tapNotes"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_below="@+id/tapStatsBadges"
android:layout_centerHorizontal="true"
android:layout_margin="16dp"

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This feels like a huge margin.

android:layout_marginTop="2dip"
android:padding="16dp"

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and padding.

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Looked ok on my tablet...but looks better with it eliminated/reduced! Fixed!

android:textAlignment="center"
android:textColor="#888"
android:textSize="24dp"
android:textStyle="italic"
android:gravity="center_horizontal" />

<ViewFlipper
android:id="@+id/illustrationFlipper"
android:layout_width="fill_parent"
android:layout_below="@id/tapStatsBadges"
android:layout_below="@id/tapNotes"
android:layout_centerHorizontal="true"
android:layout_margin="16dip"
android:layout_height="fill_parent">
Expand All @@ -72,22 +87,6 @@
android:id="@+id/tapIllustration"
android:layout_width="fill_parent"
android:layout_height="fill_parent"/>

</ViewFlipper>

<TextView
android:id="@+id/tapNotes"
android:layout_width="match_parent"
android:layout_height="fill_parent"
android:layout_alignParentBottom="true"
android:layout_below="@+id/tapStatsBadges"
android:layout_centerHorizontal="true"
android:layout_margin="16dp"
android:layout_marginTop="5dip"
android:padding="16dp"
android:textAlignment="center"
android:textColor="#888"
android:textSize="24dp"
android:textStyle="italic"/>

</RelativeLayout>
</RelativeLayout>
4 changes: 2 additions & 2 deletions kegtab/src/main/res/values-sw700dp/styles.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@
</style>

<style name="badgeValue">
<item name="android:textSize">64sp</item>
<item name="android:textSize">50sp</item>
<item name="android:layout_width">wrap_content</item>
<item name="android:layout_height">wrap_content</item>
<item name="android:textStyle">bold</item>
<item name="android:textIsSelectable">false</item>
</style>

<style name="badgeCaption">
<item name="android:textSize">18sp</item>
<item name="android:textSize">16sp</item>
<item name="android:layout_width">wrap_content</item>
<item name="android:layout_height">wrap_content</item>
<item name="android:textIsSelectable">false</item>
Expand Down
9 changes: 9 additions & 0 deletions kegtab/src/main/res/values/styles.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@
<item name="android:textColor">@color/muted</item>
</style>

<style name="beverageDetails">
<item name="android:layout_width">wrap_content</item>
<item name="android:layout_height">wrap_content</item>
<item name="android:ellipsize">end</item>
<item name="android:singleLine">true</item>
<item name="android:textSize">22sp</item>

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was a touch too big for the default font sizes on my nexus 7. ABV and IBU lines bled together. 20sp looked better to me.

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sounds good, should also look fine on my 10" tablet and will give just a bit extra room for Beer Name/Tap Name. Definitely tricky trying to get a solution that works on all screen sizes. I need to refresh my memory on the units 'sp'/'dip'. Maybe there's a better way ensure consistent UI.

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sp is supposed to be the better way at least according to what android studio just told me. :)

<item name="android:textStyle">bold</item>
</style>

<style name="jumboText">
<item name="android:layout_width">wrap_content</item>
<item name="android:layout_height">wrap_content</item>
Expand Down
21 changes: 21 additions & 0 deletions kegtab/src/main/res/xml/settings_general.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@
android:summaryOn="Temperatures will be shown in Celsius"
android:title="Show Temperatures in Celsius">
</CheckBoxPreference>
<CheckBoxPreference
android:title="Show ABV when zero"
android:key="config:ABV_DISPLAY_WHEN_ZERO"
android:summaryOn="ABV will be displayed even when zero"
android:summaryOff="ABV will not be displayed when zero"
android:defaultValue="false">
</CheckBoxPreference>
<CheckBoxPreference
android:title="Show IBU when zero"
android:key="config:IBU_DISPLAY_WHEN_ZERO"
android:summaryOn="IBU will be displayed even when zero"
android:summaryOff="IBU will not be displayed when zero"
android:defaultValue="false">
</CheckBoxPreference>
<CheckBoxPreference
android:title="Show Tap Notes"
android:key="config:DISPLAY_TAP_NOTES"
android:summaryOn="Tap Notes will be displayed if available"
android:summaryOff="Tap Notes will not be displayed"
android:defaultValue="true">
</CheckBoxPreference>

@patfreeman patfreeman May 19, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should probably be in the User Interface section of settings_kegerator.xml

@johnnyruz johnnyruz May 19, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hmm, yeah I could see it making more sense on that page since that grouping (currently only one item) controls the tap detail display. Makes sense under the title of "Look and Feel", but maybe that should just be "Units" and the UI settings are under Kegerator > User Interface

</PreferenceCategory>

</PreferenceScreen>