8000 Notifier logging by etpinard · Pull Request #4464 · plotly/plotly.js · GitHub
[go: up one dir, main page]
Skip to content
  • Insights

    • Notifier logging #4464

    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

    Jump to bottom
    Merged
    etpinard merged 6 commits into from
    Jan 6, 2020
    Merged

    Notifier logging #4464

    etpinard merged 6 commits into from
    Jan 6, 2020

    Conversation

    @etpinard
    Copy link
    Contributor
    @etpinard etpinard commented Jan 6, 2020 •
    edited
    Loading

    resolves #4098 by implementing what was proposed in #4098 (comment)

    Demos:

    Updated in #4464 (comment)

    I kept the config option name onGraphLogging to start this PR. The name notifyOnWarnings has also been proposed, but since the same config option applies to both "logs" and "warnings", maybe we could do better. Please let me know if you can think of a better option name or if you think notifyOnWarnings is the way to go.

    @archmoj @nicolaskruchten

    @etpinard
    fix logging config option declaration
    95da2b1 
    - set valType to 'integer'
- set min and max
    @etpinard
    add onGraphLogging config option
    3547173 
    - add option to notifier such that it stays on-graph
  w/o fading out
- within lib/loggers, call notifier with log/warn/error messages
    @etpinard
    call Lib.error before throwing error for no-token/missing-token issues
    e4245a6 
    ... so that the error message can show up on the graph when
    the onGraphLogging config option is turned on.
    @etpinard
    reduce transition duration for 'stick' notifier notes
    b0cf1d0
    @etpinard
    add treemap_with-without_values_template mock to flaky list
    75a7414
    @etpinard etpinard added this to the v1.52.0 milestone Jan 6, 2020
    @nicolaskruchten
    Copy link
    Contributor
    nicolaskruchten commented Jan 6, 2020

    notifyOnLogging ?

    @nicolaskruchten
    Copy link
    Contributor
    nicolaskruchten commented Jan 6, 2020

    logToNotifications ?

    @archmoj
    Copy link
    Contributor
    archmoj commented Jan 6, 2020

    viewAlert ?

    @etpinard
    Copy link
    Contribu 8000 tor Author
    etpinard commented Jan 6, 2020

    notifyOnLogging

    I'm a fan of this. Is everyone in @plotly/plotly_js ok with this?

    @antoinerg
    Copy link
    Contributor
    antoinerg commented Jan 6, 2020

    notifyOnLogging

    I'm a fan of this. Is everyone in @plotly/plotly_js ok with this?

    I'm fine with it.

    @archmoj
    Copy link
    Contributor
    archmoj commented Jan 6, 2020

    @etpinard this PR is ready to 💃
    Would you please update the codepens as well as description using the final attribute name?
    Thank you in advance!

    @etpinard
    Copy link
    Contributor Author
    etpinard commented Jan 6, 2020

    New demos with notifyOnLogging:

    @etpinard etpinard merged commit ec2a31e into master Jan 6, 2020
    @etpinard etpinard deleted the onGraphLogging branch January 6, 2020 21:16
    @jdamiba jdamiba mentioned this pull request Jan 9, 2020
    Closed
    @snyk-bot snyk-bot mentioned this pull request Feb 4, 2020
    Merged
    @snyk-bot snyk-bot mentioned this pull request Feb 22, 2020
    Open
    @snyk-bot snyk-bot mentioned this pull request Mar 18, 2020
    Open
    @snyk-bot snyk-bot mentioned this pull request Apr 17, 2020
    Open
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    feature something new

    Projects

    None yet

    Milestone

    v1.52.0

    Development

    Successfully merging this pull request may close these issues.

    On-figure error for mapbox-token-related problems

    5 participants

    @etpinard @nicolaskruchten @archmoj @antoinerg
    0