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

fix(webapp): tag selector overflow #1055

Merged
merged 5 commits into from
Apr 27, 2022
Merged

fix(webapp): tag selector overflow #1055

merged 5 commits into from
Apr 27, 2022

Conversation

pavelpashkovsky
Copy link
Contributor

@pavelpashkovsky pavelpashkovsky commented Apr 22, 2022

FIX: #1043
DEMO:
image

2 important things

  1. <input> was changed to <textarea>, I use react-textarea-autosize for correct resizing query textarea. This npm module was already injected to Pyroscope as submodule before.
  2. I had to transform query font to monospace, this is the easiest and the most accurate approach to keep the same size of symbols inside of <textarea> and <code> and keep them matching each other.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 22, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
webapp/public/assets/app.js 500.36 KB (+0.26% 🔺) 10.1 s (+0.26% 🔺) 3 s (+9.72% 🔺) 13 s

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #1055 (8c69f42) into main (d1b1547) will decrease coverage by 0.70%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1055      +/-   ##
==========================================
- Coverage   71.67%   70.97%   -0.69%     
==========================================
  Files          85       85              
  Lines        2894     2921      +27     
  Branches      618      624       +6     
==========================================
- Hits         2074     2073       -1     
- Misses        817      845      +28     
  Partials        3        3              
Impacted Files Coverage Δ
packages/pyroscope-flamegraph/src/format/format.ts 70.55% <0.00%> (-20.45%) ⬇️
...graph/src/FlameGraph/FlameGraphComponent/index.tsx 84.69% <0.00%> (-0.13%) ⬇️
packages/pyroscope-models/src/profile.ts 91.67% <0.00%> (ø)
packages/pyroscope-models/src/flamebearer.ts 60.72% <0.00%> (ø)
...raph/src/FlameGraph/FlameGraphComponent/Header.tsx 90.91% <0.00%> (ø)
webapp/javascript/services/base.ts 93.48% <0.00%> (+1.01%) ⬆️

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 d1b1547...8c69f42. Read the comment docs.

@pavelpashkovsky
Copy link
Contributor Author

pavelpashkovsky commented Apr 22, 2022

@eh-am I adjusted query input overflow, but solution isn't ideal, because at the same time this UI element is input and code. I'd appreciate any ideas on it
image

@pavelpashkovsky pavelpashkovsky requested a review from eh-am April 22, 2022 14:54
Copy link
Collaborator

@eh-am eh-am left a comment

Choose a reason for hiding this comment

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

Nice!

Just out of curiosity, did you try solving this with CSS only?
Somehow making the <code>element having the same width as the input box.
Then we don't have to do the

style={{ width: `calc(${inputWidth}px - 0.75em)` }}

part, which seems a little bit finicky.

But yeah it doesn't seem too trivial, it probably requires creating a wrapper around both the input and the code part, so that they both share the same width, then making one of them position: absolute.

@eh-am
Copy link
Collaborator

eh-am commented Apr 26, 2022

/create-server

Copy link
Collaborator

@eh-am eh-am left a comment

Choose a reason for hiding this comment

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

Looks quite good for me! One thing I noticed is that ctrl+enter is not submitting the query.

webapp/javascript/components/TagsBar.tsx Show resolved Hide resolved
@pavelpashkovsky
Copy link
Contributor Author

pavelpashkovsky commented Apr 26, 2022

@eh-am

I noticed is that ctrl+enter is not submitting the query.

It works in the same way as at https://demo.pyroscope.io/. UI detects if user changed query string inside of input, if so, it allows user submitting by ctrl+enter. Should I add possibility to submit all requests despite if they didn't change?

UPD: You are right, just found the issue.

UPD2: This issue is fixed

@Rperry2174 Rperry2174 merged commit f7c7917 into main Apr 27, 2022
@Rperry2174 Rperry2174 deleted the tag-selector-overflow branch April 27, 2022 19:29
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.

Tag selector text overflows when screen is too small
3 participants