MSD-LIVE / issues

0 stars 0 forks source link

minor issues found that need patched from latest release #173

Open zguillen opened 9 months ago

zguillen commented 9 months ago
  1. When navigating between tabs on the project details page, a new entry is added to the browser's history every time you click to a new tab. As a result, when you try to go back, you have to click back through all your tab changes. There is a browser API which allows you to modify the current history entry instead of adding a new one. Various parts of the React Router API could be used to accomplish this (search for "replace").
  2. When you update a project's logo, the logo does not refresh when you hit submit. You need to refresh the page to see the new logo.
  3. Color contrast for the yellow color on the metrics page is too low. It does not conform to accessibility requirements. The whole color pallet should probably be updated (we also need to guarantee that the color pallet is color blind friendly).
  4. The tooltip on the graphs jumps around a lot. Carina suggested fixing the tooltip to a single location.
  5. review lib used to calculate total stored (uploaded) as the 204 TB shown is inaccurate. (see comment)
zguillen commented 9 months ago

I took the json from the api response to https://data.msdlive.org/api/metrics/uploads and ran this script against it:

#!/bin/bash

# Parse the JSON file and sum the totalbytes
total_bytes=$(jq -r '[.totalbytes[] | to_entries | .[] | select(.key != "day") | .value] | add' api_response.json)

# Convert the bytes to a human-readable format
human_readable=$(numfmt --to=iec-i --suffix=B --format="%.5f" $total_bytes)

echo "Total bytes: $total_bytes"
echo "Human readable: $human_readable"

and this was printed (185 TB is the expected and correct amount) Total bytes: 204054459464956 Human readable: 185.58645TiB

jugovimm commented 9 months ago

I took the json from the api response to https://data.msdlive.org/api/metrics/uploads and ran this script against it:

#!/bin/bash

# Parse the JSON file and sum the totalbytes
total_bytes=$(jq -r '[.totalbytes[] | to_entries | .[] | select(.key != "day") | .value] | add' api_response.json)

# Convert the bytes to a human-readable format
human_readable=$(numfmt --to=iec-i --suffix=B --format="%.5f" $total_bytes)

echo "Total bytes: $total_bytes"
echo "Human readable: $human_readable"

and this was printed (185 TB is the expected and correct amount) Total bytes: 204054459464956 Human readable: 185.58645TiB

For this, the issue was simply assuming we should use SI units rather than IEC units file sizes. Do we prefer using IEC instead? SI units are used more often for non-technical folks, as well as in the file explorer

zguillen commented 9 months ago

The metrics reported to Casey have historically been in whatever units AWS reports in so he was the one who immediately noticed the difference. Apparently AWS uses SI and according to AI SI units are more appropriate since it's what's used for file sizes (SI more often for network speeds/data throughput).

jugovimm commented 9 months ago

If AWS reports in SI, I'm surprised there's a discrepancy as we were using SI before. I'll revert my commit switching us to IEC (I haven't pushed the commit up yet)

zguillen commented 9 months ago

oops, idk what aws uses but I think you just need to change whatever your UI is using to the other unit to match aws's :)

jugovimm commented 9 months ago

Gotcha. I was able to run your script on my local branch and using IEC on the metrics page gave the same output as your script. I'll push up my change using IEC

jugovimm commented 9 months ago

Here's the change I'm thinking is best for the tooltip movement:

image

Unfortunately, there aren't many good placement options for the tooltip for a variety of reasons. (1) Moving the tooltip to the left or right of the graph requires more horizontal screen space some users may not have, (2) moving the tooltip above or below the graph would conflict visually with other elements, and (3) placing the tooltip within the graph covers up data

I think going with point 3 and reducing the font size a bit is best since the user can still hover over and see data in the tooltip which the tooltip itself is covering up, and with less space taken up with the reduced font size: (1) less data is covered up, and (2) the tooltip is less prone to going off the graph (ex. when we have 12+ projects to list in a datasets tooltip, etc.)

jugovimm commented 9 months ago

Finally, what about the below for the graph color palette?

image

Since the tooltip no longer connects a particular color with a project via text color, the colors of each bar are immediately meaningless to the user as there is no key (though they could try to figure out which color is which project if they really wanted to). This means we do not need a palette that has a number of colors equal to our number of projects.

Because of this, we can choose whatever colors we think would meet the following criteria:

  1. Look good and fit best with our theme
  2. Imply there are multiple values (i.e. projects) that make up the bar
  3. Are color-blind friendly

In the example above, I've used the five colors in our theme.ts, including values from palette.primary and palette.secondary, to create a gradient using the vis4 website I linked you (link), as well as reset the opacity of each color to 1 (i.e. no transparency).

image

I think these blues fit our theme well, allow for users to detect that there are multiple values rather than one, and the website marks them as color-blind friendly. Checking the colors against the white background, two of the lighter ones are marked as warning, but do not go below a 3:1 contrast, which is fine for "big text" and assumingly thick lines like bars. The only concern is that the color will repeat, though that would happen with any palette and a high enough number of projects.

zguillen commented 9 months ago

I really like it! I'm not sure how important it is for Casey to distinguish projects in this chart though, want to run it past him? Hum...he's out so lets get Carina's approval and then we'll just push it to prod and can change colors later if Casey needs it.