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.

Sign up for GitHub

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

Jump to bottom

Ensures service details custom buttons collapse to kebab when > 3 #1394

Merged
merged 1 commit into from
Mar 1, 2018
Merged

Ensures service details custom buttons collapse to kebab when > 3 #1394

merged 1 commit into from
Mar 1, 2018

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Mar 1, 2018 •
edited
Loading

fixes https://bugzilla.redhat.com/show_bug.cgi?id=1550833

continuation of https://bugzilla.redhat.com/show_bug.cgi?id=1524718

We might need a new bz for this... as the above one was to verify for resource details page... 🤔

on master, service details page, when with custom buttons looks like this

screen shot 2018-03-01 at 1 13 17 pm

this work collapses these buttons when they exceed 3

screen shot 2018-03-01 at 1 13 39 pm

to match current behavior observed in the resource details page

screen shot 2018-03-01 at 12 01 57 pm

@AllenBW AllenBW requested review from chalettu and himdel March 1, 2018 18:33
chalettu
chalettu approved these changes Mar 1, 2018
View reviewed changes
Copy link
Contributor

@chalettu chalettu left a comment

Choose a reason for hiding this comment

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

Looks like a good change

himdel
himdel reviewed Mar 1, 2018
View reviewed changes
client/app/services/service-details/service-details.html Outdated
<custom-button
ng-if="vm.hasCustomButtons(vm.service) <= 3 && vm.hasCustomButtons(vm.service) > 0"
class="custom-dropdown pull-left"
service-id="vm.vmDetails.service.id"
Copy link
Contributor

@himdel himdel Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is a surprise.. why is vmDetails needed with 1-3 buttons but not before or with 4+?
(Looks like vmDetails is only ever used in resource-details.html, not service-details.html - bad copypasta? :))

(otherwise LGTM :))

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, woops, that one slipped through..

bad copypasta indeeeeed 😋

@AllenBW AllenBW force-pushed the enhancemet/service-details-custom-button-kebab branch from 16680a4 to 3c7e740 Compare March 1, 2018 19:42
@miq-bot
Copy link
Member

miq-bot commented Mar 1, 2018

Checked commit AllenBW@3c7e740 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

himdel
himdel approved these changes Mar 1, 2018
View reviewed changes
Copy link
Contributor

@himdel himdel left a comment

Choose a reason for hiding this comment

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

LGTM now 👍

@himdel
Copy link
Contributor

himdel commented Mar 1, 2018

@AllenBW What's the sui process here with ux/review please?

Does ux review happen asynchronously as in ui-classic or are we supposed to wait for an explicit ACK? :)

(In other words, should I punch it or wait? :))

serenamarie125
serenamarie125 approved these changes Mar 1, 2018
View reviewed changes
Copy link

@serenamarie125 serenamarie125 left a comment

Choose a reason for hiding this comment

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

Thanks for pulling this solution to service details! LGTM!

@AllenBW
Copy link
Member Author

AllenBW commented Mar 1, 2018

cc @Yadnyawalkya

@himdel himdel merged commit 928fb99 into ManageIQ:master Mar 1, 2018
@himdel himdel added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 1, 2018
@AllenBW AllenBW deleted the enhancemet/service-details-custom-button-kebab branch March 1, 2018 20:26
@Yadnyawalkya
Copy link
Member

I have created separate Bugzilla for this issue! Here you go - https://bugzilla.redhat.com/show_bug.cgi?id=1550833
cc @AllenBW @chalettu

@AllenBW
Copy link
Member Author

AllenBW commented Mar 2, 2018

cc @simaishi we now have a bz for this to be backported with... <3

simaishi pushed a commit that referenced this pull request Mar 8, 2018
@himdel @simaishi
Merge pull request #1394 from AllenBW/enhancemet/service-details-cust…
fcc0088 
…om-button-kebab

Ensures service details custom buttons collapse to kebab when > 3
(cherry picked from commit 928fb99)

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

simaishi commented Mar 8, 2018

Gaprindashvili backport details:

$ git log -1
commit fcc00885e12282f5262c84bfdc3ba13757cd2a52
Author: Martin Hradil <himdel@seznam.cz>
Date:   Thu Mar 1 20:14:16 2018 +0000

    Merge pull request #1394 from AllenBW/enhancemet/service-details-custom-button-kebab
    
    Ensures service details custom buttons collapse to kebab when > 3
    (cherry picked from commit 928fb9907590dd07253e32d1bb92c8d9cde4f81c)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553482

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serenamarie125 serenamarie125 serenamarie125 approved these changes

@himdel himdel himdel approved these changes

@chalettu chalettu chalettu approved these changes

Assignees

@himdel himdel

@simaishi simaishi

Labels
bug enhancement gaprindashvili/backported ux/approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@AllenBW @miq-bot @himdel @Yadnyawalkya @simaishi @chalettu @serenamarie125