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

Configuration - notify backend when quad_truncate gets changed #4942

Merged
merged 2 commits into from
Nov 20, 2018
Merged

Configuration - notify backend when quad_truncate gets changed #4942

merged 2 commits into from
Nov 20, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Nov 19, 2018

without miqSelectPickerEvent, any change in quad_truncate never gets propagated to the backend
=> adding miqSelectPickerEvent

introdued in #4034

Also made sure that indent of the javascript always matches the indent of the select itself,
fixed style issues (double quotes to single quotes, added semicolons),
removed useless gettext around field names (not descriptions),
and made sure miqInitSelectPicker ends up getting called exactly once.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1650461

Cc @skateman

@himdel
Copy link
Contributor Author

himdel commented Nov 19, 2018

(without the cleanup, the diff is:

+              :javascript
+                miqSelectPickerEvent('quad_truncate', '#{url}');

)

@skateman
Copy link
Member

I'm always getting complaints that I am creating too big commits 😉 could be this separated into two?

@kbrock
Copy link
Member

kbrock commented Nov 20, 2018

Thanks @himdel

without `miqSelectPickerEvent`, any change in `quad_truncate` never gets propagated to the backend
=> adding `miqSelectPickerEvent`

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1650461
made sure that indent of the javascript always matches the indent of the select itself,
fixed style issues (double quotes to single quotes, added semicolons),
removed useless gettext around field names (not descriptions),
and made sure `miqInitSelectPicker` ends up getting called exactly once.

https://bugzilla.redhat.com/show_bug.cgi?id=1650461
@himdel
Copy link
Contributor Author

himdel commented Nov 20, 2018

Rebased and split :)

@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/eaa3dfc2085cb2378a65a6384f6edc706f3cd09f~...8c98e8fdbcee7221d8a3ee45dcc60470b71aba93 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@mzazrivec mzazrivec self-assigned this Nov 20, 2018
@mzazrivec mzazrivec added this to the Sprint 100 Ending Dec 3, 2018 milestone Nov 20, 2018
@mzazrivec mzazrivec merged commit 29734e6 into ManageIQ:master Nov 20, 2018
@himdel himdel deleted the quad-event-bz1650461 branch November 20, 2018 12:25
simaishi pushed a commit that referenced this pull request Nov 20, 2018
Configuration - notify backend when `quad_truncate` gets changed

(cherry picked from commit 29734e6)

https://bugzilla.redhat.com/show_bug.cgi?id=1650461
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 7c7808600cbe01b8627268e2d608d0a290cf1497
Author: Milan Zázrivec <[email protected]>
Date:   Tue Nov 20 12:31:32 2018 +0100

    Merge pull request #4942 from himdel/quad-event-bz1650461
    
    Configuration - notify backend when `quad_truncate` gets changed
    
    (cherry picked from commit 29734e6ddc0de970dc133570d7252aee62741e8a)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1650461

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

Successfully merging this pull request may close these issues.

6 participants