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

Add edit functionality for generic object UI #11815

Merged
merged 1 commit into from
Jan 10, 2017

Conversation

eclarizio
Copy link
Member

@eclarizio eclarizio commented Oct 11, 2016

https://www.pivotaltracker.com/story/show/128427247

More details to follow when I un-wip, just wanted this out there for visibility while I'm away on jury duty. 😢

@@ -14,6 +14,22 @@
*/
function subscribeToSubject() {
ManageIQ.angular.rxSubject.subscribe(function(event) {
if (event.eventType === 'sendCountSelectedAndUpdateToolbar') {
this.MiQToolbarSettingsService.countSelected = event.countSelected;
Copy link
Contributor

Choose a reason for hiding this comment

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

Once ManageIQ/ui-components#33 is in, this should be

if (event.eventType === 'sendCountSelectedAndUpdateToolbar') {
  this.MiQToolbarSettingsService.setCount(event.countSelected);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

..and I wonder if the name could be a bit simpler .. like updateToolbarCount or something.. WDYT?

_this.MiQToolbarSettingsService.enableToolbarItemByCountSelected(item);
})
.value();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

also.. an elseif here, to keep this consistent..

@himdel
Copy link
Contributor

himdel commented Oct 13, 2016

@eclarizio upgrading manageiq-ui-components to 0.0.8 should give you that setCount now..

@eclarizio
Copy link
Member Author

Couple of screenshots to show the new "edit" drop down that is totally angularized!:

screen shot 2016-10-20 at 11 36 32 pm
screen shot 2016-10-20 at 11 36 22 pm

Thanks to @himdel's suggestions for utilizing the countSelected method from the ui-components repo in order to activate/deactivate the edit button when needed.

@miq-bot assign @gmcculloug

@gmcculloug Please review or tag someone for review. I should be able to get another PR in for the delete before vacation, but it may be WIP and can either wait until I get back or someone else can pick it up.

Reminder that this is all still "locked" behind a feature setting so if you can't see the "generic object" main tab, then you'll have to enable it.

@himdel
Copy link
Contributor

himdel commented Oct 21, 2016

Thanks to @himdel's suggestions for utilizing the countSelected method from the ui-components repo in order to activate/deactivate the edit button when needed.

🙇‍♂️ .. but note that you should update the ui-components dependency to ~0.0.8, otherwise it works by accident :). (Or wait till #12078 since this is still WIP.)

@eclarizio eclarizio changed the title [WIP] Add edit functionality for generic object UI Add edit functionality for generic object UI Oct 21, 2016
@eclarizio
Copy link
Member Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Oct 21, 2016
@gmcculloug
Copy link
Member

Looks good.

@syncrou Please review.

@miq-bot
Copy link
Member

miq-bot commented Nov 2, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@@ -44,6 +55,11 @@ def tree_data

private

def update_model_fields(generic_object_definition)
generic_object_definition.update_attribute(:name, params[:name])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is update_attributes not workable here? - e.g. is there validation that we're trying to get around? - Not sure on the history here - but the 2 separate calls to update_attribute stick out as not common.

@eclarizio
Copy link
Member Author

After rebasing, the ability to get to the generic objects UI was broken because of #12025. The removal of building a toolbar by class has been restored and the existing method has been refactored.

@romanblanco Since you were the one who removed the ability to build a toolbar by class and changed the other method names, we can talk more about the naming of the methods I re-added if you want. For now, though, I would prefer if we can build toolbars by passing the class directly, instead of some kind of standardized name that then gets magically figured out.

@miq-bot
Copy link
Member

miq-bot commented Dec 22, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@eclarizio
Copy link
Member Author

Now that all of the UI stuff has moved to the classic UI repo, this only has one change within the db/fixtures/miq_product_features.yml so that the toolbar still contains the edit button.

@miq-bot
Copy link
Member

miq-bot commented Jan 5, 2017

Checked commit eclarizio@9c6730d with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. ⭐

@chessbyte chessbyte removed the wip label Jan 9, 2017
@gmcculloug gmcculloug merged commit 95e002b into ManageIQ:master Jan 10, 2017
@gmcculloug gmcculloug added this to the Sprint 52 Ending Jan 16, 2017 milestone Jan 10, 2017
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