Improvements after jquery update by NicoWohlfarth · Pull Request #3183 · apache/cloudstack

NicoWohlfarth · 2019-02-15T14:26:51Z

Description

Fixed issues occurring with this PR
Added jQuery ui CSS to the project and adapt existing stylings
Removed unnecessary styling definitions from cloudstack.css
Swap no longer recommended jQuery method

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

We deployed the changes to our test environment and tested the UI functionality by clicking through the affected views

- remove unnecessary (empty) style definitions

- set minimum heigth of dialog

…jQuery 3.0

svenvogel · 2019-02-15T20:22:47Z

@GabrielBrascher @borisstoyanov @rhtyd @rafaelweingartner can you check please 👍

svenvogel · 2019-03-12T10:32:45Z

@wido @GabrielBrascher @borisstoyanov @rhtyd @rafaelweingartner can anybody add a ui symbol and review it?

svenvogel · 2019-03-12T10:42:03Z

this pr fixes the following problems.

svenvogel · 2019-03-12T10:44:54Z

svenvogel · 2019-03-12T10:48:42Z

DaanHoogland · 2019-03-12T12:49:32Z

@NicoWohlfarth Can you explain what these pictures are making clear? not all of them are selfevident.
@rafaelweingartner aren't these bugfixes, instead of enhancements?
LGTM overall, thanks @NicoWohlfarth .

rafaelweingartner · 2019-03-12T12:50:43Z

@DaanHoogland I selected the wrong label. Thanks Daan.

DennisKonrad · 2019-03-13T10:10:53Z

This is how it looks now

PaulAngus · 2019-03-14T13:13:52Z

@blueorangutan package

blueorangutan · 2019-03-14T13:14:04Z

@PaulAngus a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2019-03-14T14:11:57Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2635

PaulAngus · 2019-03-14T14:57:23Z

@blueorangutan test

blueorangutan · 2019-03-14T14:58:04Z

@PaulAngus a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2019-03-14T22:20:05Z

Trillian test result (tid-3424)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 24476 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3183-t3424-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Smoke tests completed. 67 look OK, 3 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	149.49	test_privategw_acl.py
test_04_rvpc_privategw_static_routes	`Failure`	265.72	test_privategw_acl.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	336.16	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	73.82	test_vpc_redundant.py
test_02_VPC_default_routes	`Failure`	731.64	test_vpc_router_nics.py

borisstoyanov

Hi @NicoWohlfarth and thanks for this fix.
I've noticed the arrow down scrolling at templates in the deploy VM wizard is only working when you actually click on the dot in the UI, if you click on the text of the template and press the arrow down you won't be able to scroll.

NicoWohlfarth · 2019-03-18T13:18:02Z

Hi @borisstoyanov.
I can't reproduce your indicated behavior even without my changes.
Without my changes you still would need to select the radio button directly to be able to navigate between the options with the arrow keys.

borisstoyanov · 2019-03-18T13:20:28Z

@NicoWohlfarth if you click on the text you select the option, but you're not able to navigate with arrows, if you click on the radio button you'll be able to use the arrows, that's what I see on my Chrome for Mac.

svenvogel · 2019-03-22T10:15:23Z

@borisstoyanov We are speaking about it internally and i come back ;)

svenvogel

fix the alignment of the radio buttons

fix the vertical alignment if the boxes and text

fix the graphic of the slider

- center buttons of create form modals - fix width of select menu in service offerings - fix width of modal in Network -> VPC -> configure -> static NAT -> aquire new Ip -> enable static nat

PaulAngus · 2019-04-10T13:14:44Z

@blueorangutan package

blueorangutan · 2019-04-10T13:15:06Z

@PaulAngus a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2019-04-10T13:35:43Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2691

PaulAngus · 2019-04-10T14:30:07Z

Manually checked the changes LGTM - can we get this merged. Even if it is not perfect, it is better, so lets merge and then everyone can add further fixes on top..

cc @borisstoyanov @rhtyd

yadvr · 2019-04-11T08:05:16Z

@PaulAngus let me review this later today/tomorrow, several css rules have been removed which needs checking as they could potentially add regression.

svenvogel · 2019-04-11T09:31:04Z

@PaulAngus @rhtyd should be ok. We create after that a new one to add another changes. We changed ui to a jquery library to make it easier in the future.

svenvogel

svenvogel

svenvogel

svenvogel · 2019-04-15T08:44:36Z

@rhtyd some news about that? further changes we would make a new pr. ;)

yadvr · 2019-04-15T09:01:59Z

@svenvogel I think we may accept the changes based on manual testing. cc @borisstoyanov - can you help with testing when you've time this week, thanks.

@svenvogel in your last reply you hinted that you're creating a new UI library? Can you elaborate, is it already part of this PR?

svenvogel · 2019-04-17T09:34:54Z

@rhtyd Hi Rohit, yes the the library are always included in cloudstack because they are a part of JQuery UI. we took a upgrade for any month #3069. Yes please look over it so that we can merge it. we will make a new PR for new changes.

@PaulAngus Can you approve it too?

DennisKonrad · 2019-04-17T11:22:44Z

We are using this for weeks now and the library update works just fine. LGTM

@rhtyd Nearly all of the UI fixes here are not caused by the jQuery update. We will fix them anyway but in another PR. This one is for the jQeury libraries.

I think this one is very good now :)

DennisKonrad · 2019-04-17T11:25:11Z

@svenvogel in your last reply you hinted that you're creating a new UI library? Can you elaborate, is it already part of this PR?

No new UI library. We try to get the existing code into good shape. Therefor the jQuery library updates.

svenvogel · 2019-04-17T11:50:21Z

@svenvogel in your last reply you hinted that you're creating a new UI library? Can you elaborate, is it already part of this PR?

No new UI library. We try to get the existing code into good shape. Therefor the jQuery library updates.

@DennisKonrad i wrote that already above!

svenvogel · 2019-04-18T11:37:34Z

@NicoWohlfarth if you click on the text you select the option, but you're not able to navigate with arrows, if you click on the radio button you'll be able to use the arrows, that's what I see on my Chrome for Mac.

#3183 (comment)

@borisstoyanov Hi Boris, this we will do in a another pr. its the best like Paul said to merge and we create new ones for other changes.

@GabrielBrascher can you take also a look on this?

DennisKonrad · 2019-04-23T09:41:34Z

@borisstoyanov Do you agree on fixing the "arrow down scrolling in templates" in another PR? If yes could you re-review?

We would like to get this merged. We will fix problems one by one.

rafaelweingartner · 2019-04-25T19:09:30Z

People discussed, reviewed and approved the PR.

It is pure JS and CSS changes. Therefore, the integration tests (that test Java code) are not relevant in this context.

Thank you all for the reviews and code changes

Nico Wohlfarth added 5 commits February 15, 2019 14:47

Add jQuery ui stylings to the project

0d23d02

Adapt stylings to jquery ui stylings

a7b26eb

- remove unnecessary (empty) style definitions

Check if jquery autocomplete is initialized before destroying it

a9d2ec1

Use dialog modal option instead of setting z-index styling

4009533

- set minimum heigth of dialog

Swap removeAttr() for prop() since it's not longer recommended as of …

b076ee2

…jQuery 3.0

svenvogel approved these changes Feb 16, 2019

View reviewed changes

Fix reccuring snapshots modal view

37cc20d

rafaelweingartner assigned NicoWohlfarth Mar 12, 2019

rafaelweingartner added type:enhancement component:UI labels Mar 12, 2019

rafaelweingartner added type:bug and removed type:enhancement labels Mar 12, 2019

GabrielBrascher added this to the 4.13.0.0 milestone Mar 12, 2019

DennisKonrad mentioned this pull request Mar 13, 2019

ui: fix computer diagram css margin that blocks down arrow #3225

Merged

5 tasks

Further style improvements

47b61f3

borisstoyanov requested changes Mar 18, 2019

View reviewed changes

More improvements on the instance-wizard (and multi-wizard in general)

1f613f4

svenvogel reviewed Mar 22, 2019

View reviewed changes

Nico Wohlfarth added 3 commits April 8, 2019 11:18

Raise input hints over modal z-index

a4d9bbd

Adapt/fix stylings

096e366

- center buttons of create form modals - fix width of select menu in service offerings - fix width of modal in Network -> VPC -> configure -> static NAT -> aquire new Ip -> enable static nat

Adapt width of dynamic-input container

5d8ec87

svenvogel reviewed Apr 15, 2019

View reviewed changes

svenvogel approved these changes Apr 15, 2019

View reviewed changes

DennisKonrad approved these changes Apr 18, 2019

View reviewed changes

rafaelweingartner merged commit 89d5480 into apache:master Apr 25, 2019

NicoWohlfarth deleted the improvements_after_jquery_update branch June 7, 2019 06:19

Conversation

NicoWohlfarth commented Feb 15, 2019

Description

Types of changes

How Has This Been Tested?

Uh oh!

svenvogel commented Feb 15, 2019

Uh oh!

svenvogel commented Mar 12, 2019

Uh oh!

svenvogel commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenvogel commented Mar 12, 2019

Uh oh!

svenvogel commented Mar 12, 2019

Uh oh!

DaanHoogland commented Mar 12, 2019

Uh oh!

rafaelweingartner commented Mar 12, 2019

Uh oh!

DennisKonrad commented Mar 13, 2019

Uh oh!

PaulAngus commented Mar 14, 2019

Uh oh!

blueorangutan commented Mar 14, 2019

Uh oh!

blueorangutan commented Mar 14, 2019

Uh oh!

PaulAngus commented Mar 14, 2019

Uh oh!

blueorangutan commented Mar 14, 2019

Uh oh!

blueorangutan commented Mar 14, 2019

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

NicoWohlfarth commented Mar 18, 2019

Uh oh!

borisstoyanov commented Mar 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenvogel commented Mar 22, 2019

Uh oh!

svenvogel left a comment

Choose a reason for hiding this comment

Uh oh!

PaulAngus commented Apr 10, 2019

Uh oh!

blueorangutan commented Apr 10, 2019

Uh oh!

blueorangutan commented Apr 10, 2019

Uh oh!

PaulAngus commented Apr 10, 2019

Uh oh!

yadvr commented Apr 11, 2019

Uh oh!

svenvogel commented Apr 11, 2019

Uh oh!

svenvogel left a comment

Choose a reason for hiding this comment

Uh oh!

svenvogel left a comment

Choose a reason for hiding this comment

Uh oh!

svenvogel left a comment

Choose a reason for hiding this comment

Uh oh!

svenvogel commented Apr 15, 2019

Uh oh!

yadvr commented Apr 15, 2019

Uh oh!

svenvogel commented Apr 17, 2019

Uh oh!

DennisKonrad commented Apr 17, 2019

Uh oh!

DennisKonrad commented Apr 17, 2019

Uh oh!

svenvogel commented Apr 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

svenvogel commented Mar 12, 2019 •

edited

Loading

borisstoyanov commented Mar 18, 2019 •

edited

Loading

svenvogel commented Apr 17, 2019 •

edited

Loading