-
Notifications
You must be signed in to change notification settings - Fork 7
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
Wrong line color on Probabilistic FSA Exceedance Curve #121
Comments
Great catch @tieganh! And thank you @drotheram for the encouragement! Fortunately, the fix is relatively easy, thanks to the clean code and documentation by @phil-evans. Many thanks, Phil! The proposed changes are:
Here is the proposed code change: diff --git a/site/assets/themes/fw-child/resources/js/rp_risks.js b/site/assets/themes/fw-child/resources/js/rp_risks.js
index 166f1e4..99887c9 100644
--- a/site/assets/themes/fw-child/resources/js/rp_risks.js
+++ b/site/assets/themes/fw-child/resources/js/rp_risks.js
@@ -1855,8 +1855,8 @@ var color_ramp = [
plugin_settings.map.layers.fsa = new L.GeoJSON(source, {
style: {
fill: false,
- color: '#000000',
- weight: 2,
+ color: '#FF0000',
+ weight: 4,
opacity: 0.6
},
pane: 'fsa'
diff --git a/site/assets/themes/fw-child/template/risks/detail.php b/site/assets/themes/fw-child/template/risks/detail.php
index 43de8ea..753ceac 100644
--- a/site/assets/themes/fw-child/template/risks/detail.php
+++ b/site/assets/themes/fw-child/template/risks/detail.php
@@ -119,7 +119,7 @@
<div id="detail-exceedance-collapse" class="collapse" data-parent="#scenario-detail-indicators" aria-labelledby="detail-exceedance-head">
<div class="card-body">
- <p>Loss exceedance curve data for postal code <strong data-indicator="fsauid"></strong>, as outlined in <span class="text-primary">red</span> on the map.</p>
+ <p>Loss exceedance curve data for postal code <strong data-indicator="fsauid"></strong>, as enclosed in the <span class="text-primary">thick red outline</span> on the map.</p>
<div id="risk-detail-chart" class="chart"></div>
@@ -131,4 +131,4 @@
</div>
</div>
-</div>
\ No newline at end of file
+</div> I have uploaded these changes to https://www.riskprofiler.ca/risks/index.html#eDt_Collapse. Please take a look and comment! For example, if you find the red outline is now too thick, I'd be happy to reduce its |
Looks good to me. Please wait for @tieganh to review before closing |
Looks good. Thanks for fixing it! |
This is how it looks after fixing this issue (#121) in OpenDRR/h7-riskprofiler@1e5e61b and Issue #125 in OpenDRR/h7-riskprofiler@6e5fdcf: |
The new version looks great, thanks @anthonyfok ! |
Thanks for your review, @tieganh! With the fix propagated into our source code in OpenDRR/h7-riskprofiler@1e5e61b, we can close this issue now. |
Describe the bug
In the screenshot it says it’s showing the exceedance probability curve for forward sortation area V8M, as outlined in “red”, but V8M is actually the thing outlined in black. The way I got to this screen was by selecting the red outlined area (SA) and then clicking the dropdown for the EP curve.
Screenshots
![Screen Shot 2023-11-06 at 4 19 14 PM](https://private-user-images.githubusercontent.com/59665171/281201789-510685e1-32eb-4080-85c4-0fba15584dcc.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk0ODg3NTEsIm5iZiI6MTczOTQ4ODQ1MSwicGF0aCI6Ii81OTY2NTE3MS8yODEyMDE3ODktNTEwNjg1ZTEtMzJlYi00MDgwLTg1YzQtMGZiYTE1NTg0ZGNjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjEzVDIzMTQxMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTJmMTAzNWFiNDU4OGIxMjZlOTRmMzQ0YmIwOTk5ZTZjYTM2ZWI0ZjA4M2M3YWY2MDQwNTA1NDQ2MzY0ZjQ2M2YmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.aX0cZ2BvXtWOsv4NXbI7aHfIHTgdS3HkC1VdXpXzCK0)
To Reproduce
Steps to reproduce the behavior:
Expected behavior
As described in the text under 'Loss Exceedance Curve', the red line should be for the FSA.
Desktop (please complete the following information):
Additional context
You can confirm that the black line is for the FSA and not the SA by selecting another SA within the black line to see that it shares the same FSA.
The text was updated successfully, but these errors were encountered: