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

fix: datepicker not updating on outside value change #441

Merged
merged 2 commits into from
Jul 15, 2021
Merged

fix: datepicker not updating on outside value change #441

merged 2 commits into from
Jul 15, 2021

Conversation

Lalaluka
Copy link
Contributor

@Lalaluka Lalaluka commented Jul 8, 2021

Hi 🧑‍💻

This is a simple fix for: #440

Added a watcher to the Value for outside changes of value.
Since the hasValue State doesn't seem to have reactivity on its own.

I'm not absolutely keen if StencilJS has some way to make States reactive directly, if there is a possibility to handle it directly in the State that might be nicer. But I couldn't find such functionality.

Can be tested like this:

<scale-date-picker label="Helper Text" id="Test" value=""></scale-date-picker>
<script>
  setTimeout(()=>{
    document.getElementById('Test').setAttribute("value", "2020-12-31")
  }, 6000)
</script>

This also enables Stuff like this:

setTimeout(()=>{
  document.getElementById('Test').setAttribute("value", "2020-12-31")
}, 6000)
setTimeout(()=>{
  document.getElementById('Test').setAttribute("value", "")
}, 8000)
setTimeout(()=>{
  document.getElementById('Test').setAttribute("value", "2020-12-31")
}, 10000)

Let me know what you think :)

@Lalaluka Lalaluka requested a review from eeegor as a code owner July 8, 2021 11:46
Copy link
Collaborator

@acstll acstll left a comment

Choose a reason for hiding this comment

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

This is amazing @Lalaluka, thank you very much for this fix!

I left a picky comment, if you're so kind.

Thanks again!

@acstll
Copy link
Collaborator

acstll commented Jul 9, 2021

By the way, @State() in Stencil is "reactive": it will trigger a render whenever it changes.

From the docs:

Any changes to a @State() property will cause the components render function to be called again.

Is that what you had doubts about?

@Lalaluka
Copy link
Contributor Author

Lalaluka commented Jul 9, 2021

Hey @acstll ,
yea State is reactive in the way that the Component is rerendered if State is changing.

What I meant was going in the direction of reference. But the longer I think about the less reasonable it is.
You know how in js Objects are references?

let a = {a: 1}
let b = a
a = {a: 2}
console.log(b) // {a: 2}

Something like that, but for the state value set.
What I didn't think about was that variables are not and state is a variable :)
Watch is exactly the tool for this issue tho.

Don't mind the comment, It is not that smart as I initially thought 😅

@Lalaluka Lalaluka requested a review from acstll July 9, 2021 12:44
Copy link
Collaborator

@acstll acstll left a comment

Choose a reason for hiding this comment

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

Thank you once more!

@acstll acstll merged commit cb5773c into telekom:main Jul 15, 2021
@Lalaluka Lalaluka deleted the fix--watchValue branch July 15, 2021 15:36
acstll pushed a commit that referenced this pull request Jul 19, 2021
* chore: implement password protected staging server, tidy up scripts (#434)

* feat: badge (#394)

* feat: init badge

* refactor: everything

* feat: init hbs

* refactor: inline style to css class mapping

* refactor: deleting global variables

* refactor: removing border

* refactor: deleting old test cases

* added storybook

* Storybook & Handlebar done

* copied and added the index.html from main branch

* prettier & lint

* fixed errors according to Daniel check,  after merge request 1

* format and lint

* changes commited according to Daniels check from first MR

* applied change according to Daniel, from merge request 2.

* design tokens added

* Font-Family fixed

* added design token for font-family

* Edited storybook - Font

Co-authored-by: marvinLaubenstein <[email protected]>

* ci(bernerslee): remove s3 pipeline

* Sketch/readonly input state (#373)

* feat(sketch): add readonly states to text-fields

* feat(sketch): add readonly states for textarea

* feat(textarea): add readonly styles

* refactor(sketch): add readonly to smybol name state resolver

* chore(sketch): update sketch symbol DB

* feat: implement scaleBeforeClose event in Modal (#430)

* Update includes and symbol names (#372)

* chore: update static sketch symbols

* chore: update color swatches and missing readonly state

* refactor: change symbol hirarchy for textarea / text-field

* refactor: update readonly assignments

* fix(button): add icon scaling and resiziable layout of slot elements

* fix(dropdown/sketch): improve rendering for select element

* chore: autoformat files

* fix: datepicker not updating on outside value change (#441)

* refactor(rating-stars): rework with input range approach

also make the whole thing more modular with parts

* feat(rating-stars): add half star rating

* feat(rating-stars): use correct aria value and add better half visuals

* refactor: add size, editable label; css

* feat: only first star can clear all stars

* refactor: prop renaming; remove watcher;

* test: writing tests

* refactor: renaming value to rating

* refactor: stories

* feat: implement onTouchEnd

* feat(rating-stars): add readonly state and a11y features

* feat(rating-stars): simplify revert logic / handle minRating input

* feat: add prop readonly

* feat: edit PR requests

* feat: firstStarSelected with a default value of false

* feat: input gets focus onClick

* feat(rating-stars): add sketch generation and adjust input handling

* fix: revert handmade sketch includes

* test: update snapshots

Co-authored-by: Kutlovcidenis <[email protected]>
Co-authored-by: marvinLaubenstein <[email protected]>
Co-authored-by: Daniel Beck <[email protected]>
Co-authored-by: Stefan Kopco <[email protected]>
Co-authored-by: Calvin Schröder <[email protected]>
Co-authored-by: Christian Pajung <[email protected]>
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.

2 participants