Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

fix(table): fix sorting, column width calculation, and text wrapping #253

Merged
merged 2 commits into from
Nov 13, 2019

Conversation

conglei
Copy link
Contributor

@conglei conglei commented Nov 12, 2019

💔 Breaking Changes

🏆 Enhancements

📜 Documentation

🐛 Bug Fix
This PR is to

  1. better calculation of the column width (with formatted string);
    image

  2. lexicographically sorting strings;
    image

  3. fixing word truncating issues;
    image

🏠 Internal

cc: @etr2460 @graceguo-supercat

@conglei conglei requested a review from kristw November 12, 2019 23:22
@conglei conglei requested a review from a team as a code owner November 12, 2019 23:22
@codecov
Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #253 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   37.99%   37.99%           
=======================================
  Files          12       12           
  Lines         229      229           
  Branches       21       21           
=======================================
  Hits           87       87           
  Misses        132      132           
  Partials       10       10

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14f0238...403dbf8. Read the comment docs.

function getText(value: unknown, format: TimeFormatter | NumberFormatter | undefined) {
let formattedValue = value;
if (format) {
formattedValue = format.format(value as any);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return here? If there is a format function, that should not produce html output.

let formattedValue = value;
if (format) {
formattedValue = format.format(value as any);
}
if (typeof value === 'string') {
const span = document.createElement('span');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another new trick I learn is using regex to remove all html tags.
I think it should be faster than parsing html via span tag, although haven't really tested it.

const htmlTagRegex = /(<([^>]+)>)/gi;
const text = value.replace(htmlTagRegex, '');

@netlify
Copy link

netlify bot commented Nov 12, 2019

Deploy preview for superset-ui-plugins ready!

Built with commit 403dbf8

https://deploy-preview-253--superset-ui-plugins.netlify.com

@conglei conglei merged commit 4b5cb1c into master Nov 13, 2019
@delete-merged-branch delete-merged-branch bot deleted the conglei-table-fix branch November 13, 2019 01:29
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants