feat(range): add neutral point#17400
Merged
liamdebeasi merged 13 commits intoionic-team:masterionic-team/ionic-framework:masterfrom Feb 12, 2019
KillerCodeMonkey:feat/range-neutral-pointCopy head branch name to clipboard
Merged
feat(range): add neutral point#17400liamdebeasi merged 13 commits intoionic-team:masterionic-team/ionic-framework:masterfrom KillerCodeMonkey:feat/range-neutral-pointCopy head branch name to clipboard
liamdebeasi merged 13 commits intoionic-team:masterionic-team/ionic-framework:masterfrom
KillerCodeMonkey:feat/range-neutral-pointCopy head branch name to clipboard
Conversation
Contributor
|
Would this work in RTL as well? |
Contributor
Author
|
@abennouna i did no changes to the base calculations. Maybe you can test this on your #17384 ? |
Contributor
|
@KillerCodeMonkey I guess I'll wait till #17384 is merged then test yours if it's approved. |
Contributor
Author
|
@abennouna okay :) |
liamdebeasi
suggested changes
Feb 12, 2019
liamdebeasi
reviewed
Feb 12, 2019
Contributor
|
Looks great! If you can just fix the merge conflicts, we should be good to merge! 👍 |
Contributor
Author
|
@liamdebeasi merge conflicts resolved and fixed the active bar styling :) |
Contributor
|
@KillerCodeMonkey Looks like something got changed during your merge. The tick marks on the ranges are no longer visible. This is what This is what your branch looks like: |
Contributor
Author
|
@liamdebeasi sry my fault. The any types are getting me crazy. Maybe this would be another part of improvement 😉 |
liamdebeasi
approved these changes
Feb 12, 2019
Contributor
liamdebeasi
left a comment
There was a problem hiding this comment.
Great work! Just going to wait for the tests to pass then I'll merge this in. 🎉
Contributor
|
This actually causes a regression in RTL mode. |
Contributor
|
Hi @abennouna, Would you be able to file a bug report? Thanks! |
Contributor
|
Maybe we also need RTL tests? |
Contributor
|
That would be good to have. I can discuss it with the team. |
Contributor
Author
|
@abennouna if you let me know, what you expect in the behaviour of rtl in this case i will look into it ;), but only saying it is a "regression" is a little bit unclear. |
Contributor
|
@KillerCodeMonkey the PR seems to cancel the fix from #17384 A screencast shows the difference: Before #17400: With PR #17400: By reverting the PR, range work correctly again. That's what I mean by regression. |
Contributor
Author
|
@abennouna i am working on it :) |
liamdebeasi
added a commit
that referenced
this pull request
Feb 20, 2019
This reverts commit 15acb4b. revert neutral point
Contributor
|
Hey there! After some discussion with the team, we have decided to revert your changes from this PR. We really appreciate all of the time and effort you put into creating these updates for Ionic, but we are not certain this is something we would like to introduce to the code base. Specifically, we are not certain about the use cases for this feature. The native input, material design, and polymer specs do not have this neutral point feature. Would you be able to share with us the use case(s) you had in mind for this feature? After that the team can have another discussion about this PR. Sorry for the confusion, and please don't hesitate to reach out if you have any questions! 🙂 |
liamdebeasi
added a commit
that referenced
this pull request
Feb 20, 2019
Contributor
Author
|
We have ranges from negative to positive values.
But the default is 0.
So If you Change the value you want to Show the User how this value changes
from the default. And often the Base/Default is Not the min value.
We are working in the health care and There we have such use cases quite
often, e.g. by doing diagnosis are define therapies.
Liam DeBeasi <notifications@github.com> schrieb am Mi., 20. Feb. 2019,
21:42:
… Hey there!
After some discussion with the team, we have decided to revert your
changes from this PR. We really appreciate all of the time and effort you
put into creating these updates for Ionic, but we are not certain this is
something we would like to introduce to the code base. Specifically, we are
not certain about the use cases for this feature. The native input,
material design, and polymer specs do not have this neutral point feature.
Would you be able to share with us the use case(s) you had in mind for
this feature? After that the team can have another discussion about this PR.
Sorry for the confusion, and please don't hesitate to reach out if you
have any questions! 🙂
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17400 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKOYExUjBftzk4cKx6ZP-7aq79qqdvIks5vPbMngaJpZM4alJy9>
.
|
brandyscarney
pushed a commit
that referenced
this pull request
Mar 6, 2019
Contributor
Author
|
@liamdebeasi are there hopes for a "remerge" or do you discussed it again? |
Contributor
Author
|
@liamdebeasi @brandyscarney bump :) |
Contributor
|
Hey there! Sorry for the delay - I thought I had replied to this already. (I guess not 🙂) At this time we aren't going to add this feature to the framework. Sorry for any confusion! |
liamdebeasi
pushed a commit
that referenced
this pull request
Apr 26, 2019
* feat(Range): add neutral point * feat(Range): generate proxies and api * fix(): check positive case in neutralPointChanged * fix(Range): neutralPoint to min if neutralPoint < min * fix(Range): active bar style * fix(Range): tick styling
liamdebeasi
added a commit
that referenced
this pull request
Apr 26, 2019
liamdebeasi
added a commit
that referenced
this pull request
Apr 26, 2019
* docs(ng): update angular test readme Closes #17212 * docs(breaking): add breaking changes and copy editing (#17214) - adds col auto to grid - updates overlays to point to the same usage - copy editing - adds more info on the tabs changes - documents event changes closes ionic-team/ionic-docs#258 - update nav event documentation references #16819 * fix(build): modify rollup.config.js to work with Windows (#17231) * docs(react): fix typo Closes #17243 * fix(reorder): capture click event (#17244) fixes #17241 * fix(grid): add flex to ion-grid to allow it to properly render in an ion-item (#17258) fixes #17075 * docs(segment) add example for default Value Closes #17275 * docs(alert): correct alert-controller link path (#17294) references ionic-team/ionic-docs#254 * docs(loading): correct loading-controller link path (#17295) closes ionic-team/ionic-docs#254 * fix(range): chrome bug with will-change * docs(): update links Ref #17256 * docs(): rebuild docs * docs(): rebuilding * docs(): update incorrect links * fix(searchbar): hide search icon when focused with cancel button (#17260) fixes #17252 * fix(react): duplicate events being fired in ionic/react (#17321) * docs(modal): fix typo with returning header (#17333) * test(searchbar): update searchbar tests to take focused ss (#17318) - gets focus test working properly - adds a no cancel button focus test this should prevent the regression in #17252 in the future * Merge branch 'master' into update-pwa-check merge: * Merge branch 'master' into add-mobileweb-platform merge * Merge pull request #17356 from ionic-team/add-mobileweb-platform feat(platform): add mobileweb platform * Merge branch 'master' into update-pwa-check merge * Merge pull request #17355 from ionic-team/update-pwa-check fix(platform): add additional check for safari PWA * Merge branch 'master' into add-mobileweb-platform merge * Merge pull request #17356 from ionic-team/add-mobileweb-platform feat(platform): add mobileweb platform merge * Merge branch 'master' into update-pwa-check merge and try again. * Merge pull request #17355 from ionic-team/update-pwa-check fix(platform): add additional check for safari PWA merge * docs(rtl): Fix small typo in item docs (#17365) * docs(datetime): fix typo (#17360) * docs(loading): remove dismissOnPageChange, add ionLoadingDidDismiss (#17357) * docs(loading): remove dismissOnPageChange, change onDidDismiss to ionLoadingDidDismiss * docs(loading): add usage showing events * docs(loading): update example to show proper usage * docs(rtl): Fix small typo in item docs (#17365) merge * docs(loading): update example to show proper usage docs(loading): update example to show proper usage merge and try again. * test(platform): Add Platform tests (#17354) * test(platform): add base platform tests * test(platform): add isPlatform test, clean up test file * test(platform): do not export matchMedia * test(platform): change window to win to avoid confusion * fix(menu): fix content shadow when revealed in iOS (#17383) * fix(popover): originate animation from right in RTL/MD (#17381) * fix(popover): apply fixed position to keep backdrop in viewport (#17352) fixes #17337 * 4.0.1 * fix(tab-bar): add translucent tab-bar styles back (#17376) - updates css to allow for translucent tab-bar - adds translucent test * docs(datetime): usage typos and clean up (#17415) Fix some typos, clean up a code snippet, reword slightly to use clearer language (in consideration of ESL readers). * docs(reorder): Update incomplete reorder docs (#17417) * add base reorder doc updates * update doc wording * remove extra sentence * clear up explanation sentence * fix typo * run build * fix doc definition for ionitemReorder * make requested changes * remove prop table * chore(github): update issue template * docs(loading): update breaking doc to show new loading usage (#17431) * docs(loading): add missing async to new loading example (#17432) * update loading example * add async * chore(react): release of ionic react 0.0.4 (#17442) * chore(github): update issue templates (#17433) * fix(searchbar): allow setting of toolbar color and searchbar color (#17474) * fix(searchbar): allow setting of toolbar color and searchbar color * fix test label typo * docs(popover): add missing comma in example (#17401) * fix(range): implement RTL (from PR 17157) (#17384) - MD PIN fixes not committed because they depend on mixin changes references #17012 * docs(): Add documentation for slots (#17441) * add button slot * add component slot docs * update content default slot description * run npm build * fix typos * update md files * docs(slots): update slot components and the build files * chore(stencil): updates stencil to build readmes * chore(build): update the swiper bundle file to match master * update default slot doc wording * revert changes * Revert "update default slot doc wording" This reverts commit e184014. merge * docs(slots): update default slot doc wording * docs(avatar): update angular usage for img src (#16884) * fix(config): update types for scrollPadding, inputBlurring and hideCaretOnScroll to boolean (#17302) * feat(range): add neutral point (#17400) * feat(Range): add neutral point * feat(Range): generate proxies and api * fix(): check positive case in neutralPointChanged * fix(Range): neutralPoint to min if neutralPoint < min * fix(Range): active bar style * fix(Range): tick styling * fix(range): improved rtl support (#17479) * fix(Range): rtl support * fix(Range): knob position in rtl * fix(select): Account for when options are not loaded immediately (#17405) * Added logging to begin debugging issue * identify potential fix, add test * fix(select): render when options are loaded after a delay * fix linter issues * fix e2e test * fix edge case with if statement * fix(item-sliding): Sliding no longer breaks after removing an item (#17492) * fix(item-sliding): Sliding no longer breaks after removing an item * run linter * fix(datetime): default to current date when no value given (#17443) * fix(datetime): default to current date when no value given * test(datetime): add spec test * move getDateValue to utils * docs(process): update process documentation * fix(button): show proper shade for activated button on ios (#17508) fixes #17436 * 4.0.2 * chore(range): revert neutral point (#17550) * Revert "fix(range): improved rtl support (#17479)" This reverts commit f832de5. revert range rtl support * Revert "feat(range): add neutral point (#17400)" This reverts commit 15acb4b. revert neutral point * sync in 4.0.1 and 4.0.2 * docs(): Add documentation for slots (#17441) * add button slot * add component slot docs * update content default slot description * run npm build * fix typos * update md files * docs(slots): update slot components and the build files * chore(stencil): updates stencil to build readmes * chore(build): update the swiper bundle file to match master * update default slot doc wording * revert changes * Revert "update default slot doc wording" This reverts commit e184014. merge * docs(slots): update default slot doc wording
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Short description of what this resolves:
Ability to set the neutral point of the picker. Active ticks and bar are measured from the neutral point instead of the min value.
Changes proposed in this pull request:
Ionic Version:
4.0.0