Formatting for float
and integer
columns via Intl.NumberFormat
#2563
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
integer
andfloat
columns. From #2539 which this PR supersedes:Things I changed from the predecessor PRs:
<select>
columns
.column_config
tocolumns_config
for consistency (this is the corollary for thecolumns
property andplugin
/plugin_config
).update_and_render()
. This method is confusingly named - it does not callupdate()
, it updates theVIew
config (which is very expensive) and re-renders. TheView
config 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_config
fromcolumns
) is still a weird term. The suffixconfig
is autological and fails to describe what distinguishes this field from its siblingcolumns
.plugin_config
is 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_config
is 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.