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

External plugins load after confirming config warning #1748

Merged
merged 11 commits into from Mar 3, 2021

Conversation

peterkxie
Copy link
Contributor

This closes #1692, now the config warning modal holds off fetchPlugins until after the user confirms that they trust the source, so external plugins get loaded after. Cancel resets to default configuration (same logic as onFactoryReset)

Making as draft for now because not too sure how to handle the cancel case. Assuming they do not want to load the cross origin config, will have to atleast clear the config in the URL and in the configPath and set it to the default config (config.json). If clearing the config, may have to clear the session since that session may have info recorded using the previous config (such as confirming, loading a view, and then refresh and not confirming/cancel the second time).

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Feb 27, 2021
@codecov
Copy link

codecov bot commented Feb 27, 2021

Codecov Report

Merging #1748 (e34b4c4) into master (4de4374) will decrease coverage by 0.05%.
The diff coverage is 6.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1748      +/-   ##
==========================================
- Coverage   58.80%   58.75%   -0.06%     
==========================================
  Files         454      455       +1     
  Lines       21007    21027      +20     
  Branches     4975     4975              
==========================================
+ Hits        12354    12355       +1     
- Misses       8343     8362      +19     
  Partials      310      310              
Impacted Files Coverage Δ
products/jbrowse-web/src/sessionWarningModal.tsx 12.50% <0.00%> (+5.83%) ⬆️
products/jbrowse-web/src/Loader.tsx 59.36% <5.00%> (-4.87%) ⬇️
products/jbrowse-web/src/configWarningModal.tsx 12.50% <12.50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4de4374...e34b4c4. Read the comment docs.

@cmdcolin
Copy link
Collaborator

Visiting http://localhost:3000/?config=test_data/config_demo.json has an error currently about GDCTrack not being found on this branch

@peterkxie peterkxie added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Mar 1, 2021
@peterkxie peterkxie self-assigned this Mar 1, 2021
@peterkxie peterkxie added this to To do in JBrowse team board via automation Mar 1, 2021
@rbuels rbuels moved this from To do to In progress in JBrowse team board Mar 1, 2021
@cmdcolin
Copy link
Collaborator

cmdcolin commented Mar 2, 2021

Is this one ready now?

@peterkxie peterkxie marked this pull request as ready for review March 2, 2021 20:25
@cmdcolin cmdcolin merged commit c14aca0 into master Mar 3, 2021
JBrowse team board automation moved this from In progress to Done Mar 3, 2021
@cmdcolin
Copy link
Collaborator

cmdcolin commented Mar 3, 2021

Looks good to me :)

@cmdcolin
Copy link
Collaborator

cmdcolin commented Mar 3, 2021

merge'd

@cmdcolin cmdcolin deleted the 1692_plugin_load_after_warning branch March 3, 2021 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

plugins from external sessions should not load until external-session warning is confirmed
3 participants