Formatting for float and integer columns via Intl.NumberFormat#2563
Merged
Formatting for float and integer columns via Intl.NumberFormat#2563
float and integer columns via Intl.NumberFormat#2563Conversation
initial datagrid integration shim datagrid into new format fix datagrid tests symbols integration refactor api update migrations Co-authored-by: Davis Silverman <sinistersnare@users.noreply.github.com> Co-authored-by: Broch Stilley <brochington@gmail.com> tests, lint better column_config update repr update tests
implement intl.numberFormat constructor UI rm fixed; add migration; fix tests
8910e93 to
d3e9911
Compare
This was referenced Mar 11, 2024
d3e9911 to
df569ef
Compare
df569ef to
4f44520
Compare
Closed
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
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.
Adds formatting options for
integerandfloatcolumns. From #2539 which this PR supersedes:Things I changed from the predecessor PRs:
<select>columns.column_configtocolumns_configfor consistency (this is the corollary for thecolumnsproperty andplugin/plugin_config).update_and_render(). This method is confusingly named - it does not callupdate(), it updates theVIewconfig (which is very expensive) and re-renders. TheViewconfig diffs and drops spurious updates, but it is still not behavior we should rely on, plus there is a better option inRenderer::update().There are a bunch of issues remaining with this PR that I feel need to be resolved:
columns_config(wascolumn_configfromcolumns) is still a weird term. The suffixconfigis autological and fails to describe what distinguishes this field from its siblingcolumns.plugin_configis not obviously better to my eye either. At least as a member ofplugin_config, this kept thesave()/restore()API for plugins simple. Additionally, this has been moved to share with e.g.group_by, which is another config depth we should conflate less (as some of these properties to to theView()constructor, but some must be removed).IMO we should be unwinding this ambiguity completely, not shuffling it around. This:
... is now this ...:
... but IMO this makes more sense, and mirrors how this data structure is ultimately dissected during
restore():Refactorings like this are costly on the Perspective community (and me), but this would be complex to do today as
plugin_configis untyped and the state is managed externally, so I can accept this conceit to the structure of the code for now. It is still a fairly significant breaking change to the persistence API.