Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Android Material] Linear Progress Indicator #5079

Merged
merged 1 commit into from Feb 7, 2019
Merged

[Android Material] Linear Progress Indicator #5079

merged 1 commit into from Feb 7, 2019

Conversation

paymicro
Copy link
Contributor

Description of Change

fixes Linear Progress Indicator

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Before/After Screenshots

Not applicable

Testing Procedure

  • run [Material] ProgressBar Gallery
  • check Visual Gallery

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@samhouts samhouts added this to In Review in v3.6.0 Jan 29, 2019
@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jan 29, 2019
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 148 Jan 29, 2019
@paymicro paymicro changed the base branch from master to 3.5.0 January 29, 2019 19:35
@samhouts samhouts removed this from In Review in v3.6.0 Jan 29, 2019
@samhouts samhouts added this to In Review in v3.5.0 Jan 29, 2019
@PureWeen
Copy link
Contributor

@paymicro can you rebase this to master when you get a chance?

@samhouts samhouts changed the base branch from 3.5.0 to master January 29, 2019 23:44
@samhouts samhouts added retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. and removed retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Jan 29, 2019
@PureWeen
Copy link
Contributor

build --uitests

Copy link
Contributor

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

I am not sure what the reason was to drop the appcompat and go completely custom?

I am worried that we are doing all this work, and then when we get the actual implementation from Google - which is planned - we will have a totally different look.

And then, totally separate - we are dropping the material design guidelines. The whole material thing is to be strict and when the user sets a color, this is set for the whole control. If I want a purple control, then everything is purple. Shouldn't we, as the implementors, restrict what the user can do? What is the point of material guidelines if we don't follow them? Of course, the user can do what they want...


namespace Xamarin.Forms.Platform.Android.Material
{
public class MaterialProgressBarRenderer : AProgressBar,
IVisualElementRenderer, IViewRenderer, ITabStop
{
const float BackgroundAlpha = 0.3f;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did we get this value from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same value is used in the MaterialProgressBarRenderer for iOS. To match the background by default, this value was chosen. Although in the Google Guide the value is 0.6
screenshot_16

Copy link
Contributor

Choose a reason for hiding this comment

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

Yay. Google. This is typical for Material - neither platform matches the spec.

<style name="XamarinFormsMaterialProgressBarHorizontal" parent="android:Widget.ProgressBar.Horizontal">
<item name="android:indeterminateOnly">false</item>
<item name="android:minHeight">4dp</item>
<item name="android:progressDrawable">@drawable/MaterialProgressBar</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you opt to use the base control and a custom drawing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just got reminded by Shane, Google has a HARD set for 4dp. Of course, this is absolutely not true for iOS, but hey, why follow your own guides.

@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jan 30, 2019
@mattleibow
Copy link
Contributor

@paymicro Could you attach screenshots of before/after to the issue for comparison and records (for when Google breaks the controls and the style suddenly changes under us)?

@samhouts samhouts removed the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jan 31, 2019
@samhouts samhouts added this to In Review in v3.6.0 Jan 31, 2019
@samhouts samhouts removed this from In Review in v3.5.0 Jan 31, 2019
v4.0.0 automation moved this from In Review to Done Feb 7, 2019
@mattleibow
Copy link
Contributor

Merged this into the material branch for the slider / progress as they are the same control and I want to reuse some of the coloring code. (and the nice gallery)

@samhouts samhouts added this to In Progress in v3.6.0 Feb 12, 2019
@samhouts samhouts moved this from Done to Ready for Review (Issues) in Sprint 148 Feb 12, 2019
@samhouts samhouts removed this from In Progress in v3.6.0 Feb 12, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 12, 2019
@samhouts samhouts removed this from In Progress in v3.6.0 Feb 13, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 13, 2019
@samhouts samhouts removed this from In Progress in v3.6.0 Feb 13, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 13, 2019
@samhouts samhouts removed this from In Progress in v3.6.0 Feb 13, 2019
samhouts pushed a commit that referenced this pull request Feb 13, 2019
* [Android Material] Linear Progress Indicator (#5079)

Merging into the material "slider" / "progress" bar branch so that we can share some code as they are the same control

* [Material] [Slider, ProgressBar] Updated the progress bard and added the slider

* Renamed the gallery

fixes #5008
fixes #5079
fixes #5018
@samhouts samhouts added this to In Progress in v3.6.0 Feb 14, 2019
@samhouts samhouts moved this from Ready for Review (Issues) to Done in Sprint 148 Feb 14, 2019
@samhouts samhouts moved this from In Progress to Done in v3.6.0 Feb 14, 2019
@samhouts samhouts removed this from Done in v3.6.0 Feb 14, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 15, 2019
@samhouts samhouts moved this from In Progress to Done in v3.6.0 Feb 19, 2019
@samhouts samhouts removed this from Done in v3.6.0 Feb 19, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 19, 2019
@samhouts samhouts moved this from In Progress to Done in v3.6.0 Feb 20, 2019
@samhouts samhouts removed this from Done in v3.6.0 Feb 20, 2019
@samhouts samhouts modified the milestones: 4.0.0, 3.6.0 Feb 20, 2019
@samhouts samhouts removed this from Done in v4.0.0 Feb 21, 2019
@samhouts samhouts added this to In Progress in v3.6.0 Feb 21, 2019
@samhouts samhouts moved this from In Progress to Done in v3.6.0 Feb 21, 2019
@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/visual p/Android p/iOS 🍎 retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. t/enhancement ➕
Projects
No open projects
Sprint 148
  
Done
v3.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants