Skip to content
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

Fit mode #353

Merged
merged 5 commits into from
Aug 27, 2021
Merged

Fit mode #353

merged 5 commits into from
Aug 27, 2021

Conversation

eh-am
Copy link
Collaborator

@eh-am eh-am commented Aug 26, 2021

An initial implementation for #304

(A video)
https://user-images.githubusercontent.com/6951209/131033045-c3a9acd1-2f95-4f2a-8447-287a4a099138.mp4
Screenshot from 2021-08-26 17-41-54

  • Used a monospace font to make certain calculations easier
  • implemented for the canvas/table
  • used a dropdown instead of a switch, since IMO switches are more for the on/off config (https://material.io/components/switches#theming) plus I can see we adding more modes based on heuristics in the future (for example in go we would strip the github.com prefix, I dunno)

There are a few UX things to be discussed:

  • should we save it in the (redux) store and make it affect all tabs? What about the comparison one? Should we be able to edit both at well?
  • How about grafana?

Discussion

There are few quirks with the word alignment, where you can get situations like
Screenshot from 2021-08-26 16-42-58

An option would be to fill the string with spaces so as to align, but that requires figuring out the cell size (which can be done, but I prefer to use as minimum javascript for layout as possible).
Another option would be to align right, but then depending on the width you could get weird situations such as
Screenshot from 2021-08-26 16-40-04

Hence why I think having the occasional misalignment on the left is okay.

@codecov
Copy link

codecov bot commented Aug 26, 2021

Codecov Report

Merging #353 (b794c89) into main (1e1267a) will decrease coverage by 0.56%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #353      +/-   ##
==========================================
- Coverage   54.36%   53.81%   -0.55%     
==========================================
  Files         102      103       +1     
  Lines        5013     5068      +55     
==========================================
+ Hits         2725     2727       +2     
- Misses       2039     2085      +46     
- Partials      249      256       +7     
Impacted Files Coverage Δ
pkg/agent/session.go 58.65% <0.00%> (-5.54%) ⬇️
pkg/storage/segment/timeline.go 94.74% <0.00%> (-5.26%) ⬇️
pkg/util/bytesize/bytesize.go 33.34% <0.00%> (-4.59%) ⬇️
pkg/storage/segment/key.go 68.50% <0.00%> (-3.97%) ⬇️
pkg/storage/segment/segment.go 88.28% <0.00%> (-3.50%) ⬇️
pkg/structs/transporttrie/trie.go 96.97% <0.00%> (-3.03%) ⬇️
pkg/agent/upstream/remote/remote.go 62.32% <0.00%> (-2.89%) ⬇️
pkg/storage/storage.go 63.58% <0.00%> (-1.63%) ⬇️
pkg/storage/segment/serialization.go 72.53% <0.00%> (-1.62%) ⬇️
pkg/build/build.go 2.50% <0.00%> (-0.44%) ⬇️
... and 11 more

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 5acc0b8...b794c89. Read the comment docs.

petethepig and others added 2 commits August 26, 2021 17:52
* font changes

* more font changes
@petethepig petethepig merged commit 508bdfc into main Aug 27, 2021
@petethepig petethepig deleted the 304-fit-mode branch August 27, 2021 19:37
@petethepig
Copy link
Member

Looks great! Thanks for quick turnaround! And thanks for adding the tests too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants