Feature/1807 custom confirm modal #1834
Merged
Conversation
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
ysbaddaden
approved these changes
Jan 5, 2023
Contributor
ysbaddaden
left a comment
ysbaddaden
left a comment
There was a problem hiding this comment.
Oh my, the JS on this project is really 🙈 🙉 🙊
I didn't get the window[this.props.funcName]() at first but that's because the modal is a react component, whereas the upload results is jQuery injected into the HTML.
Oh, well.
Contributor
Author
|
Yeah!! That's what has made me struggle with how to do it!! And then after talking with you I realized this was the way... But is a "sophisticated" ─to use a nice word :)─ wiring of the functionalities |
leandroradusky
added a commit
that referenced
this pull request
Jan 17, 2023
* New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Rework Box Label (#1819) * Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count * Box reports for LOD samples working * Small fix * Enabled PDF and SVG export in box reports * Visualization and print implmented for Challenge Boxes * Added computed AUC, TPR and FPR * automatic unblinding for samples with uploaded measurements (#1831) * Fixing of box label in UI (#1836) * Feature/1807 custom confirm modal (#1834) * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Upgrade to `mini_racer` gem (#1791) * Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure. * GHA: fix deprecated nodejs version warning * Styled slider, small fixes * Allow to download CSV template file and style fixes (#1835) * Allow to download CSV template file and style fixes * Fixes PR review * Basic functioning report for variants box * Merging completition * Merging completition * Merging completition * Merging competition * Fixes PR review Co-authored-by: Julien Portalier <julien@portalier.com>
leandroradusky
added a commit
that referenced
this pull request
Jan 19, 2023
* Feature/1809 samples reports entity (#1823) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Fixes PR review * Fixes PR review * Fixes PR review * Fixes Julien's PR review * SamplesReportsController specs (#1839) * Boxes reports functioning (#1840) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Rework Box Label (#1819) * Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count * Box reports for LOD samples working * Small fix * Enabled PDF and SVG export in box reports * Visualization and print implmented for Challenge Boxes * Added computed AUC, TPR and FPR * automatic unblinding for samples with uploaded measurements (#1831) * Fixing of box label in UI (#1836) * Feature/1807 custom confirm modal (#1834) * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Upgrade to `mini_racer` gem (#1791) * Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure. * GHA: fix deprecated nodejs version warning * Styled slider, small fixes * Allow to download CSV template file and style fixes (#1835) * Allow to download CSV template file and style fixes * Fixes PR review * Basic functioning report for variants box * Merging completition * Merging completition * Merging completition * Merging competition * Fixes PR review Co-authored-by: Julien Portalier <julien@portalier.com> * Merging reports into main & fixes * more bugfixes * Update app/helpers/samples_reports_helper.rb Co-authored-by: Julien Portalier <julien@portalier.com> * small fix Co-authored-by: Julien Portalier <julien@portalier.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #1807 & #1808.
When a file containing invalid IDs is uploaded a modal to confirm the upload is shown as in the mockup:
Also, a warning message is shown in a modal to confirm the measurements uploading, as in this mockup
Some notes:
red) and this same class modify the title and the text of the modal. Maybe is better to add two separate props to the modal? The name was chosen to make explicit that modifying other than the color will make the modal look not ok since the title and the content will apply the same class.ConfirmModalclass to call callback functions and use this component as a regular modal and not just as a confirm submission one (thanks Julien <3).confirmation_modal.js.jsxfile was really old and I removed the tabs to indent with spaces as in the rest of the project.