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

feat(ramp): Allow specifying ramp weights #2505

Merged
merged 4 commits into from Sep 22, 2021
Merged

feat(ramp): Allow specifying ramp weights #2505

merged 4 commits into from Sep 22, 2021

Conversation

TheDoctor314
Copy link
Contributor

@TheDoctor314 TheDoctor314 commented Sep 21, 2021

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

Adding weight attribute in ramps for more gradient-like behaviour.
It defines the size of the interval of the ramp stage.

Related Issues & Documents

Fixes #1750

Documentation (check all applicable)

  • This PR requires changes to the Wiki documentation (describe the changes)
  • This PR requires changes to the documentation inside the git repo (please add them to the PR).
  • Does not require documentation changes

We need to document the fact that ramps can have their weights specified for a particular stage

@TheDoctor314 TheDoctor314 marked this pull request as ready for review September 21, 2021 13:58
@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #2505 (c7f58e9) into master (98d9a88) will increase coverage by 0.02%.
The diff coverage is 37.50%.

❗ Current head c7f58e9 differs from pull request most recent head 9c3551f. Consider uploading reports for the commit 9c3551f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2505      +/-   ##
==========================================
+ Coverage   10.07%   10.10%   +0.02%     
==========================================
  Files         147      147              
  Lines       10229    10235       +6     
==========================================
+ Hits         1031     1034       +3     
- Misses       9198     9201       +3     
Flag Coverage Δ
unittests 10.10% <37.50%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/drawtypes/ramp.hpp 50.00% <ø> (ø)
src/drawtypes/ramp.cpp 48.71% <37.50%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98d9a88...9c3551f. Read the comment docs.

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

Nice work!

I imagined this being done by calculating which ramp element to use depending on the percentage and the different weights. But your solution is so much less complex 😄

Please also mention this in the Added section of the changelog: https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog

src/drawtypes/ramp.cpp Outdated Show resolved Hide resolved
TheDoctor314 and others added 2 commits September 22, 2021 01:00
*Add test for ramp weights

*[drawtypes/ramp] Implement ramp weights

Simply clone `label_t` weight no. of times in the icon list
This helps us not to change any of the calculations.

*Fix silly bug

Forgot to add a hyphen for the `weight` parameter.

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

*doc: add #1750 to CHANGELOG
Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

Looks good now :)

Thanks a lot for contributing 🎉

Use std::make_shared.
@patrick96 patrick96 merged commit 55eb19f into polybar:master Sep 22, 2021
@TheDoctor314 TheDoctor314 deleted the ramp-weights branch September 23, 2021 09:24
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.

ramp and animation stage weights
2 participants