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

Name the tags on the topology screens based on its classifications #3034

Merged
merged 1 commit into from
Dec 14, 2017

Conversation

skateman
Copy link
Member

@skateman skateman commented Dec 12, 2017

The name attribute on tags is displaying the full tag URI instead of a human-readable name. The tags we want to display on topology screens always provide a related Classification object with an appropriate description. This object always has a parent Classification also with the description. Using these objects we can build the desired human-readable tag name by joining the two descriptions together.

As the tag name is being displayed on a tooltip with a Name: prefix before, I did not wanted to use a second colon for joining the two descriptions. Instead of that I am dropping the prefix for tags.

Before:
screenshot from 2017-12-12 15-34-11

After:
screenshot from 2017-12-14 17-31-18

Depends on:
ManageIQ/manageiq#16607
#3007

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

@miq-bot add_label bug, pending core, gaprindashvili/yes, topology

@epwinchell
Copy link
Contributor

@miq-bot assign @dclarizio

@miq-bot miq-bot assigned dclarizio and unassigned himdel Dec 12, 2017
@epwinchell
Copy link
Contributor

epwinchell commented Dec 12, 2017

@dclarizio @skateman

From the VM Summary:
screen shot 2017-12-12 at 12 18 56 pm

@dclarizio
Copy link

Another showing multiple tags in a category:

image

@skateman skateman force-pushed the topology-tag-naming branch from c1c96c4 to 97786d3 Compare December 12, 2017 20:22
@skateman
Copy link
Member Author

@epwinchell changed it to A: B

@dclarizio
Copy link

@skateman So is Name:, Type:, and Status: a standard topology popup or could tags have their own text format? Like:

Department: Accounting | Engineering
Type: Tag

Especially, since Status has no real meaning for a tag anyway.

@skateman
Copy link
Member Author

@dclarizio I'm not saying it can't be done, but it would add extra complexity into the codebase. But if @himdel approves, I'm okay with it...

@himdel
Copy link
Contributor

himdel commented Dec 13, 2017

I'm OK with it too.. one extra if for tags won't kill us :).

@martinpovolny
Copy link
Member

Marking as WIP based on the conversation.

@martinpovolny martinpovolny changed the title Name the tags on the topology screens based on its classifications [WIP] Name the tags on the topology screens based on its classifications Dec 14, 2017
@skateman skateman force-pushed the topology-tag-naming branch from 97786d3 to 8525e06 Compare December 14, 2017 10:57
@skateman
Copy link
Member Author

Adjusted based on the suggestions, updated the screenshots...

@skateman skateman changed the title [WIP] Name the tags on the topology screens based on its classifications Name the tags on the topology screens based on its classifications Dec 14, 2017
@miq-bot miq-bot removed the wip label Dec 14, 2017
@dclarizio
Copy link

@skateman was it not easy to remove "Status" from the tag popup? They really don't have a status. Let me know, otherwise, we can merge when green.

Also removing pending core label as the depends on PRs have been merged.

@skateman skateman force-pushed the topology-tag-naming branch from 8525e06 to 44bd5a4 Compare December 14, 2017 16:48
@miq-bot
Copy link
Member

miq-bot commented Dec 14, 2017

Checked commit skateman@44bd5a4 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@skateman
Copy link
Member Author

@dclarizio updated code and screenshot

@dclarizio dclarizio merged commit d7681c0 into ManageIQ:master Dec 14, 2017
@dclarizio dclarizio added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 14, 2017
@skateman skateman deleted the topology-tag-naming branch December 14, 2017 22:04
simaishi pushed a commit that referenced this pull request Dec 15, 2017
Name the tags on the topology screens based on its classifications
(cherry picked from commit d7681c0)

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

Gaprindashvili backport details:

$ git log -1
commit 49b6b3fd89710ae6f031903d613caa0c6cf3b839
Author: Dan Clarizio <[email protected]>
Date:   Thu Dec 14 13:37:21 2017 -0800

    Merge pull request #3034 from skateman/topology-tag-naming
    
    Name the tags on the topology screens based on its classifications
    (cherry picked from commit d7681c09db29f2207040ea311332c38ffd3dbef7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1526582

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.

7 participants