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

bug #1479: Configuration UI overlaps Combined-Services-Badge and Show-Logs-Icon #1489

Merged
merged 10 commits into from
Nov 10, 2022

Conversation

sbraitsch
Copy link
Contributor

@sbraitsch sbraitsch commented Jun 28, 2022

Closes #1479
Added a max width to the name template. Name + ID combinations that are too long will get ellipsized to prevent overlap.
I also moved the service-count badge statically next to the show log and show config icons to make the position consistent.
Added the name + id as a tooltip, so it can be seen fully even when it gets ellipsized.


This change is Reviewable

@sbraitsch sbraitsch requested a review from mariusoe June 28, 2022 07:26
@MariusBrill MariusBrill self-assigned this Jul 7, 2022
@sbraitsch sbraitsch removed the request for review from mariusoe July 7, 2022 14:48
@heiko-holz
Copy link
Contributor

Will be discussed in the week of 29.08-02.09.2022 when I'm in FFM

danipaniii
danipaniii previously approved these changes Oct 17, 2022
Copy link
Contributor

@danipaniii danipaniii left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @heiko-holz and @MariusBrill)

@danipaniii
Copy link
Contributor

In order to always show the full name of the service name and not having to hover over it, there is the possibility to change the styling like this: .this :global(.might-overflow) { max-width: 19rem; display: inline-block; white-space: normal; overflow: visible; overflow-wrap: break-word; text-overflow: unset; }

@danipaniii
Copy link
Contributor

danipaniii commented Oct 31, 2022

The original solution:
original

When hovering over the name it looks like this:
original_hover

The alternative:
alternative

@heiko-holz
Copy link
Contributor

We discussed in the team and decided for the version with the line-break.
@danipaniii can you please implement and commit that version :)?

danipaniii and others added 3 commits November 4, 2022 11:54
…the full name is always displayed and before it would surpass the column box, the name continues on a new line
Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r4, 1 of 1 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MariusBrill and @sbraitsch)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 188 at r6 (raw file):

          }

          .this :global(.might-overflow):hover {

do we even need the agent name on hover?
I.e., can we remove the title for the agent name?
Let's discuss this in today's sync

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MariusBrill)


components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js line 188 at r6 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

do we even need the agent name on hover?
I.e., can we remove the title for the agent name?
Let's discuss this in today's sync

As discussed, I removed the hover attribute and related CSS class

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MariusBrill)

@heiko-holz heiko-holz merged commit b78aae6 into inspectIT:master Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration UI overlaps Combined-Services-Badge and Show-Logs-Icon
5 participants