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

Add support for showing/ hiding pins #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svkaka
Copy link

@svkaka svkaka commented Mar 17, 2019

added attribute for the pin
updated sample application

added attribute for pin
updated sample application
@svkaka svkaka changed the title added support for showing/ hiding pins Add support for showing/ hiding pins Mar 17, 2019
Copy link
Collaborator

@krazykira krazykira left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution but i think the solution is not scalable in this way rather i would suggest you update the mrb_temporaryPins attribute from boolean to enum

The following things you would need to add.

  1. Define an enum PinDisplayProperties having the values ALWAYS,NEVER,ONLY_ON_TOUCH
  2. Add way to change mrb_temporaryPins from xml.
  3. Add way to change mrb_temporaryPins from code.
  4. Update the sample app accordingly so that we have the ability to see the different properties of mrb_temporaryPins

@@ -32,6 +32,7 @@
<attr name="mrb_pinRadius" format="dimension"/>
<attr name="mrb_connectingLineColors" format="reference"/>
<attr name="mrb_onlyOnDrag" format="boolean"/>
<attr name="mrb_pinsEnabled" format="boolean"/>
Copy link
Collaborator

@krazykira krazykira Mar 18, 2019

Choose a reason for hiding this comment

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

we already have a property mrb_temporaryPins and i think this one should be changed so it should have the following three values

  1. ALWAYS (This displays them all the time)
  2. NEVER (This hides them all the time)
  3. ON_TOUCH_ONLY (This displays them when we are modifying the value and then hides them )

1 & 3 can be achieved right now by changing value for mrb_temporaryPins to true and false but the 2nd one is what you have to implement and at the same time you need to change the mrb_temporaryPins

So at the end the mrb_temporaryPins would be changed to something like this


 <attr name="mrb_temporaryPins" format="enum">
            <enum name="ALWAYS" value="0" />
            <enum name="NEVER" value="1" />
            <enum name="ON_TOUCH_ONLY" value="2" />
        </attr>

Copy link
Author

Choose a reason for hiding this comment

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

makes sense, I will add it

@krazykira
Copy link
Collaborator

@svkaka can we have an ETA on the changes. I would like to release a new version in May and this could be added if the changes are done in time

@krazykira
Copy link
Collaborator

@svkaka Please resolve conflicts as well

harishsinghania added a commit to harishsinghania/material-range-bar that referenced this pull request Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants