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

Provide built-in css classes for text badges #201

Merged

Conversation

cronik
Copy link
Contributor

@cronik cronik commented Oct 11, 2024

Restores stylized text only badges lost in 2.0 refactor. Text only badges (no icon or link) will render with rounded corners and default background color that matches the text color but with some opacity.

Before:
Screenshot 2024-10-11 at 6 11 16 PM

After:
Screenshot 2024-10-11 at 6 10 16 PM
Screenshot 2024-10-11 at 5 53 07 PM

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@cronik cronik requested a review from a team as a code owner October 11, 2024 22:15
@strangelookingnerd
Copy link
Contributor

The functionality itself was not really „lost“ but removed intentionally. I wanted to simplify the implementation of this plugin and more importantly give users full control over the badge styling. Any „implicit styling“ was therefore removed and I’m not really convinced that restoring it is a good idea.

Maybe a provocative question, but what if I actually want a text only badge without border / background?

Possibly related #177

@cronik
Copy link
Contributor Author

cronik commented Oct 13, 2024

I was able to reproduce something with a similar effect with a bunch of css

addBadge(text: "1.0.0", style: 'color: var(--green); border-radius: 5px; background: var(--light-green); padding: 0 0.4rem !important;')

Users need to dig deep into css to make a simple text tag look somewhat presentable. The end user pays the cost of the plugin simplification given the same could have been accomplished before with:

addShortText(text: '1.0.0', color: 'success')

It appears this cost transfer was intentional so as a compromise would you be ok with bundling some default css classes that can be explicitly added rather than included by default?

addBadge(text: "1.0.0", style: 'color: var(--success-color);', cssClass: 'badge-textTag')

If that sounds reasonable I can rework this PR and update documentation.

@strangelookingnerd
Copy link
Contributor

Having a set of pre-defined CSS classes sounds like a great idea. We could add those to the documentation similar to the various examples for symbols.

Add CSS classes for text only badges to simplify common styling consistent with
the Jenkins design library.
@cronik cronik force-pushed the feature/styled-text-only-badge branch from ffdf855 to f907020 Compare October 14, 2024 22:43
@cronik
Copy link
Contributor Author

cronik commented Oct 14, 2024

Refactored to include 2 text badge css classes:

  • badge-text--background - Adds colored background with rounded corners.
  • badge-text--bordered - Adds border that is the same color as the text.

Examples on how they can be combined and tweaked with the style parameter

addBadge(text: "1.0.0", style: 'color: var(--success-color);', cssClass: 'badge-text--background')
addBadge(text: "2.0.0", style: 'color: var(--warning-color);', cssClass: 'badge-text--bordered')
addBadge(text: "3.0.0", style: 'color: var(--warning-color); border-radius: 6px;', cssClass: 'badge-text--bordered')
addBadge(text: "4.0.0", style: 'color: var(--warning-color);', cssClass: 'badge-text--background badge-text--bordered')
addBadge(text: "5.0.0", cssClass: 'badge-text--background badge-text--bordered')
Screenshot 2024-10-14 at 6 47 45 PM

@mPokornyETM
Copy link
Contributor

Refactored to include 2 text badge css classes:

  • badge-text--background - Adds colored background with rounded corners.
  • badge-text--bordered - Adds border that is the same color as the text.

Examples on how they can be combined and tweaked with the style parameter

addBadge(text: "1.0.0", style: 'color: var(--success-color);', cssClass: 'badge-text--background')
addBadge(text: "2.0.0", style: 'color: var(--warning-color);', cssClass: 'badge-text--bordered')
addBadge(text: "3.0.0", style: 'color: var(--warning-color); border-radius: 6px;', cssClass: 'badge-text--bordered')
addBadge(text: "4.0.0", style: 'color: var(--warning-color);', cssClass: 'badge-text--background badge-text--bordered')
addBadge(text: "5.0.0", cssClass: 'badge-text--background badge-text--bordered')
Screenshot 2024-10-14 at 6 47 45 PM

it will be fine, when you bring this examplesalso in the 'official' (readme) documentation

Copy link
Contributor

@mPokornyETM mPokornyETM left a comment

Choose a reason for hiding this comment

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

LGTM

@strangelookingnerd strangelookingnerd changed the title Stylize text only badge Provide built-in css classes for text badges Oct 15, 2024
@strangelookingnerd strangelookingnerd merged commit f94fb19 into jenkinsci:master Oct 15, 2024
20 checks passed
@strangelookingnerd
Copy link
Contributor

I’ll wait for #200 to be resolved before releasing this change. Thanks for the contribution @cronik

@strangelookingnerd strangelookingnerd added enhancement New feature or request documentation Documentation improvements labels Oct 15, 2024
@jimklimov
Copy link
Contributor

jimklimov commented Oct 24, 2024

FWIW, in my use-cases I went with a CSS file that can be served from $JENKINS_HOME/userContent and requested via theme plugins (also, several CSS files can be served by one including them), and applying one of several classes depending on the message and its severity, e.g.

...this did partially start out from an attempt to recreate the Badge v1.9.x experience with the newer plugin releases, which this PR I think largely restores as a choosable option :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants