Support design library symbols and colors

cronik commented 4 months ago

Allow badges to use Jenkins symbols and color references. Update info/warn/error badges to use symbols by default.

Testing done

Used run-fast to create badges used in the screenshots.

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

cronik commented 4 months ago

Thanks for those enhancements.

The reason for adding color as a constructor instead of a setter was down to the fact that all the sub classes of AddBadgeStep don't support the color parameter, but the docs generator would include it. So in the interest of keeping the changes to a min I went the constructor route. Another option might be to adjust the Step type hierarchy a bit to get everything to align properly. I'll test some stuff out, maybe it's not as much additional change as I had originally thought.

cronik commented 4 months ago

@MarkEWaite I managed to rework the PR to move the color parameter to a setter and only apply it to the generic addBadge step. Was able to revert the doc generator changes as result.

MarkEWaite commented 4 months ago

Thanks @cronik! I saw that there was no online help for the addBadge parameters, so I added online help for them and for a few other items.

Since the plugin is up for adoption, this pull request would be a great one for you to use to adopt the plugin. Are you interested in adopting the plugin? I'm happy to continue as a reviewer and tester, since I use the badge plugins and I maintain the embeddable build status plugin.

If you are willing to adopt the plugin, I think it would be good for us to take the step to switch to automated plugin release so that the plugin is released each time a new enhancement is merged.

MarkEWaite commented 4 months ago

The online help that I added is working as expected on the addBadge step for all parameters except the color parameter. I don't understand why the color parameter behaves differently in the online help than the help for the icon, id, link, and text parameters. Unfortunately, I've run out of ideas to explore. I've left the source file for the text help in the pull request, even though it is not displayed to the user from the "Pipeline syntax" page.

I think that it is an improvement to have online help, even if the help is not displayed for the color parameter. The color parameter is well documented in the README.

The online help is visible from the "/pipeline-syntax" page when the addBaadge entry is selected in the dropdown menu and the "?" icon is clicked that is next to each parameter. That text will be extracted into the "Pipeline steps reference" page after this pull request is merged and released.

In case you'd like to see how it looks, here is a screenshot of the addBadge online help from the Pipeline syntax snippet generator:

MarkEWaite commented 4 months ago

While testing the combined pull request:

133

I discovered that var(--yellow) does not change the icon color. There is an example in the README that says:

addBadge(icon: 'symbol-star', text: 'A star', color: 'var(--yellow)')

Can you help me identify the mistake I am making when using that example?

I see a similar surprise with the example:

addBadge(icon: 'symbol-star', text: 'A star', color: 'rgb(239, 245, 66)')

I would be just fine removing those examples if they are not expected to change the color of the symbol.

MarkEWaite commented 4 months ago

The createSummary behavior has changed from the 1.9.1 release to this pull request. The word "span" is incorrectly inserted into the message when I use appendText as suggested in one of the examples.

Before this change

When I use the following Pipeline:

def summary = createSummary(icon: 'green')
summary.appendText('Completed text added.', true)
summary.appendText(' Bold text added.', false, true, false, 'black')

After this change

MarkEWaite commented 4 months ago

The createSummary behavior has changed from the 1.9.1 release to this pull request. The word "span" is incorrectly inserted into the message when I use appendText as suggested in one of the examples.

Fixed in 3b4c000544457dac93c3dda249a4c82976a66641

cronik commented 4 months ago

@MarkEWaite Thanks for the collaboration on this PR!

I discovered that var(--yellow) does not change the icon color. There is an example in the README that says:

One issue is that the color style is not set on l:icon. But after some additional testing and digging I realized that even if <l:icon style="${badgeStyle}" is set, l:icon does not respect it when the icon path starts with symbol-. So I think some revised docs are in order to indicate that only Jenkins palette and semantic color names and classes are support for symbol icon references. For other icon references those css references should work as expected with the update I just pushed.

It should be noted that var(--yellow) works with addShortText. I updated the help and docs to make this more clear.

cronik commented 4 months ago

Since the plugin is up for adoption, this pull request would be a great one for you to use to adopt the plugin. Are you interested in adopting the plugin? I'm happy to continue as a reviewer and tester, since I use the badge plugins and I maintain the embeddable build status plugin.

@MarkEWaite I'm not entirely sure what it means to adopt a plugin, but I suppose I could.

MarkEWaite commented 4 months ago

@MarkEWaite I'm not entirely sure what it means to adopt a plugin, but I suppose I could.

Adopting a plugin means that you're willing to become a maintainer of the plugin. Plugin maintainers review pull requests, review issue reports, merge pull requests they have reviewed, and release new versions.

The frequently asked questions should address many questions. @strangelookingnerd just submitted the email message and pull request to become a maintainer. I suspect that @strangelookingnerd would be happy to have an additional maintainer to help with code reviews and other maintenance work.

The plugin needs more modernization as described in the "Improve a plugin" tutorial. It also needs to have automated release enabled so that bug fixes and enhancement requests are automatically released when they are merged.

MarkEWaite commented 4 months ago

@strangelookingnerd thanks very much for adopting the plugin. I'm willing to merge and release it if needed, but would prefer that you would do that as a new maintainer. I'm happy to answer questions or provide other encouragement as needed.

strangelookingnerd commented 4 months ago

@MarkEWaite I took the freedom of adding my remark from https://github.com/jenkinsci/badge-plugin/pull/130#discussion_r1590258007 to this PR. This is by not appending plugin-ionicons-api to the icon name (in case they start with symbol-) and rather expect the icon name to follow the pattern of symbol-icon-name plugin-plugin-name as noted in https://weekly.ci.jenkins.io/design-library/Symbols. I also updated the docs and pipeline syntax help. Let me know what you think and I'll gladly merge and release this PR.

MarkEWaite commented 4 months ago

@MarkEWaite I took the freedom of adding my remark from #130 (comment) to this PR. This is by not appending plugin-ionicons-api to the icon name (in case they start with symbol-) and rather expect the icon name to follow the pattern of symbol-icon-name plugin-plugin-name as noted in https://weekly.ci.jenkins.io/design-library/Symbols. I also updated the docs and pipeline syntax help. Let me know what you think and I'll gladly merge and release this PR.

That looks great to me. I made minor changes to the documentation. I think it is ready to be squash merged and released.

Thanks @cronik for the pull request and thanks @strangelookingnerd for adopting the plugin

jenkinsci / badge-plugin