-
Notifications
You must be signed in to change notification settings - Fork 7
Truncating institution name on box label #1819
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
Merged
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
Contributor
|
@leandroradusky please read the issue again, it says "without label, truncate to 22 characters" :) |
Contributor
|
Also consider that I'm proposing another change: remove the |
Contributor
|
This PR should be merged and included in a new release based on https://github.com/instedd/cdx/releases/tag/0.21.5 |
Contributor
Author
ysbaddaden
approved these changes
Dec 12, 2022
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 #1816
Institution name is now truncated for institution name to avoid a new line that broke the label.
Before:
Now:
Given that an ellipsis is used for truncating, there are only 6 characters displayed when the institution name exceeds the 9 characters, which is relatively short, maybe
Institutioncan be shortened? (cc @diegoliberman)