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

Facelift ThreadGroup UI, improve alignment of Name+Commments fields for all components #513

Merged
merged 1 commit into from
Oct 5, 2019

Conversation

vlsi
Copy link
Collaborator

@vlsi vlsi commented Oct 2, 2019

Description

This is a draft, however, it enables us to preview how aligned fields might look like.

Note: I did not touch radio-buttons yet.

Screenshots (if appropriate):

Before:
JMeter 5.2.0-SNAPSHOT, thread group ui

After:

Снимок экрана 2019-10-04 в 15 43 15

Снимок экрана 2019-10-04 в 15 44 36

@ham1
Copy link
Contributor

ham1 commented Oct 2, 2019

Much nicer already :)

@ThomasArdal
Copy link

ThomasArdal commented Oct 3, 2019

Looks awesome. I would have left-aligned Thread group, Name, and Comments with the borders below. And decreased the top margin to make more room for content. It might just be a personal opinion. Basically like on the Before shot.

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 3, 2019

Just in case: the labels in the "scheduler configuration" section miss colons.

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 3, 2019

And decreased the top margin to make more room for content

Indeed. I've removed the extra margin and I've updated "after" image.

@pmouawad
Copy link
Contributor

pmouawad commented Oct 4, 2019

Hi Felix,
If this has been tested on Windows , Linux and MacOSX and on HDMI screen, then you should commit it.
Or do you need to work on it ?

On what platforms did you test so that we test on others ?
How could we work together on this ?

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 4, 2019

@pmouawad , I'm still updating the UI.

How could we work together on this ?

Do you intend to implement the related changes as well?

I suggest as follows: I'll finish with Tread Group, commit it, then we could revise other components. Ok?

@pmouawad
Copy link
Contributor

pmouawad commented Oct 4, 2019 via email

@vlsi vlsi changed the title WIP: Align fields on Thread Group UI Facelift ThreadGroup UI, improve alignment of Name+Commments fields for all components Oct 4, 2019
@vlsi vlsi force-pushed the mig_threadgroup branch 4 times, most recently from 9b13ec7 to 8f24766 Compare October 4, 2019 13:13
@vlsi
Copy link
Collaborator Author

vlsi commented Oct 4, 2019

@pmouawad , I've updated the PR, and it looks good to me now (provided Travis tests pass).
It would be great if someone could try the change. I've tested it with macOS.

@ThomasArdal , I wonder what would you think of the removal of "Scheduler Configuration" section.

@codecov-io
Copy link

codecov-io commented Oct 4, 2019

Codecov Report

Merging #513 into master will decrease coverage by 0.01%.
The diff coverage is 95.45%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #513      +/-   ##
============================================
- Coverage     56.16%   56.15%   -0.02%     
+ Complexity    10017    10016       -1     
============================================
  Files          1024     1024              
  Lines         62913    62897      -16     
  Branches       7064     7064              
============================================
- Hits          35337    35321      -16     
- Misses        25101    25104       +3     
+ Partials       2475     2472       -3
Impacted Files Coverage Δ Complexity Δ
.../main/java/org/apache/jmeter/gui/CommentPanel.java 72.22% <ø> (-27.78%) 2 <0> (-3)
...he/jmeter/visualizers/RespTimeGraphVisualizer.java 50.95% <0%> (ø) 18 <0> (ø) ⬇️
...apache/jmeter/visualizers/StatGraphVisualizer.java 56.81% <0%> (ø) 25 <0> (ø) ⬇️
...org/apache/jmeter/threads/AbstractThreadGroup.java 84.78% <100%> (ø) 22 <1> (ø) ⬇️
...src/main/java/org/apache/jmeter/gui/NamePanel.java 86.48% <100%> (+0.37%) 13 <1> (+1) ⬆️
...rg/apache/jmeter/control/gui/LoopControlPanel.java 77.92% <100%> (+0.89%) 15 <3> (+3) ⬆️
...che/jmeter/threads/gui/AbstractThreadGroupGui.java 47.52% <100%> (ø) 9 <1> (ø) ⬇️
.../org/apache/jmeter/threads/gui/ThreadGroupGui.java 92.7% <100%> (-1.65%) 14 <3> (-3)
.../main/java/org/apache/jmeter/util/JMeterUtils.java 48.29% <100%> (+0.41%) 70 <1> (+1) ⬆️
.../apache/jmeter/gui/AbstractJMeterGuiComponent.java 92.64% <95.83%> (+5.34%) 18 <4> (-1) ⬇️
... and 3 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 3e2dda1...80b830e. Read the comment docs.

@pmouawad
Copy link
Contributor

pmouawad commented Oct 4, 2019

Hi @vlsi ,
I am not in favor of:

  • removing the warning even with renaming
  • switching to checkbox instead of radio button, I find it less clear. You have in mind that every tester knows that if it's unchecked it means "Different user for each iteration" which is not the case IME

@ThomasArdal
Copy link

I wonder what would you think of the removal of "Scheduler Configuration" section.

Not sure what that is? Or where it is?

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 4, 2019 via email

@ThomasArdal
Copy link

I don't think that I have ever used those fields.

@ubikloadpack
Copy link
Contributor

I don't think that I have ever used those fields.

Did you use 3rd party Thread Group to configure duration of test ? or did you only use iterations ?
Or something else ?

When you look at new UI, does it look more clear in terms of what each property means ?

Thanks for your help

@ubikloadpack
Copy link
Contributor

Should loop count be renamed to "Iterations" ?

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 4, 2019

Should loop count be renamed to "Iterations" ?

It is reused from Loop Controller.
Do you suggest to rename the label there?
Do you suggest to use different labels for the same thing?

@vlsi
Copy link
Collaborator Author

vlsi commented Oct 4, 2019

What do you think of renaming Stop Test Now to Halt test?

@ubikloadpack
Copy link
Contributor

Should loop count be renamed to "Iterations" ?

It is reused from Loop Controller.
Loop Controller is more of a "programatic element", I think user would understand
Do you suggest to rename the label there?
but if it's renamed it will be clear in both cases
Do you suggest to use different labels for the same thing?
As it's not good, iterations would be maybe more clear

@ubikloadpack
Copy link
Contributor

Halt test

Force stop ?

@vlsi vlsi force-pushed the mig_threadgroup branch 2 times, most recently from cef435a to a50c1bd Compare October 4, 2019 22:40
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.

6 participants