Skip to content

Conversation

@raviks789
Copy link
Contributor

@raviks789 raviks789 commented Sep 13, 2021

resolves #303

requires Icinga/ipl-web#63

@cla-bot cla-bot bot added the cla/signed label Sep 13, 2021
@raviks789 raviks789 force-pushed the fix/redesign-state-badge-widget branch from bb43418 to af19f35 Compare September 13, 2021 14:20
Copy link
Contributor

@flourish86 flourish86 left a comment

Choose a reason for hiding this comment

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

I'm sorry, the original mockup still was done with the legacy theme.

Not sure, if it's just me. But compared to the mockups this doesn't look quite right, yet.

Here's a screenshot from how it looks in my Browser.

Screen Shot 2021-09-21 at 15 26 43

Could you please change the following:

  • The Total count should be moved into the badge
  • Please remove the dark transparent background
  • The badges or badge groups

We should also combine handled and not-handled objects of the same state into a badge-group, like it has been done in the footer of the icingadb-web lists (e.g. Hosts).

Please take a look:
Screen Shot 2021-09-21 at 15 43 42

We should also combine the badges by color (e.g. CRITICAL/DOWN) as this is otherwise hard to grasp, especially when there's many different badges.

Here's another mockup with the dark mode:

Artboard Copy 5

If you have any further questions, feel free to ask. 😊

@raviks789 raviks789 force-pushed the fix/redesign-state-badge-widget branch 10 times, most recently from 6af4e49 to 655898c Compare September 23, 2021 08:23
@raviks789 raviks789 marked this pull request as ready for review September 23, 2021 08:25
@raviks789 raviks789 force-pushed the fix/redesign-state-badge-widget branch 2 times, most recently from 3486e7a to f07a189 Compare September 23, 2021 09:42
@raviks789 raviks789 force-pushed the fix/redesign-state-badge-widget branch from 82f0e6e to 74b4841 Compare September 28, 2021 12:01
Copy link
Contributor

@flourish86 flourish86 left a comment

Choose a reason for hiding this comment

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

Please address the mentioned changes, otherwise it looks fine.👌 Thanks!

@raviks789 raviks789 force-pushed the fix/redesign-state-badge-widget branch from 74b4841 to 4444733 Compare September 28, 2021 15:15
Copy link
Contributor

@flourish86 flourish86 left a comment

Choose a reason for hiding this comment

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

Looks perfect to me. Thanks! 👍

@lippserd lippserd added this to the 2.4.0 milestone Nov 22, 2021
@nilmerg nilmerg force-pushed the fix/redesign-state-badge-widget branch from 4444733 to a9ec1b7 Compare December 17, 2021 12:14
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

I've ensured compatibility with v2.10 now and cleaned up a bit. Note that Icinga/ipl-web#63 is required for this to work now.

Though, I believe some changes in the UI are not thought through enough:

  • The total children count is placed at a weird location. The count belongs to the direct children. It's not the total of all states that are shown on the right. Best to be seen in the lower left tile. I think this confuses users an we'll get certainly reports that this number is too low. I think we should place the total count where it was previously.
  • The OK states we're not shown previously. Now if there's everything green, like in the tiles below, they look somewhat off to me. I'd propose to not show them like before.

Screenshot from 2021-12-17 13-44-31

@flourish86
Copy link
Contributor

As discussed in our meeting, we should change the badges counts to the following:

  • We will only show problems, so everything, but ok/up
  • We will refer to nodes (Host/Service/Process) for the states, so there will be max. one badge for every state
  • As a consequence the sum of the state badges items will be max. the number of the total amount.
  • To clarify this we label the total number of node “[n] Nodes

This is how this would then for example look in the end.

Artboard Copy 6

@flourish86 flourish86 force-pushed the fix/redesign-state-badge-widget branch from 10760c9 to f0b4e9d Compare January 17, 2022 09:53
@flourish86 flourish86 requested a review from nilmerg January 17, 2022 09:56
@nilmerg nilmerg force-pushed the fix/redesign-state-badge-widget branch from f0b4e9d to 464967b Compare March 4, 2022 13:24
@flourish86 flourish86 force-pushed the fix/redesign-state-badge-widget branch from 464967b to 641b469 Compare March 9, 2022 14:00
@flourish86 flourish86 requested a review from nilmerg March 9, 2022 14:01
@flourish86 flourish86 force-pushed the fix/redesign-state-badge-widget branch 3 times, most recently from 86e6b71 to a45980a Compare March 10, 2022 10:21
@flourish86 flourish86 force-pushed the fix/redesign-state-badge-widget branch from a45980a to d325844 Compare March 10, 2022 11:02
@nilmerg nilmerg merged commit 5ccc13c into master Mar 10, 2022
@nilmerg nilmerg deleted the fix/redesign-state-badge-widget branch March 10, 2022 11:24
@nilmerg nilmerg added the enhancement New feature or improvement label Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed enhancement New feature or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redesign state badge widget on tiles

5 participants