homepage
homepage copied to clipboard
Feature: Add list view for custom api
Proposed change
Closes #2872
Currently, the custom API widget is limited to displaying up to four label-value pairs horizontally, I am here proposing having a list view that extends vertically.
The added feature is 100% compatible with old services.yaml files. For the above example, the only difference in config between two widgets is that one has specified to have a list view.
I know the list view exceeds the 1 row 4 block widget guideline, but it is not unprecedented as it is directly copied from the Coin Market Cap widget, and the custom API widget is the perfect place to make exceptions for more customization.
Type of change
- [ ] New service widget
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Documentation only
- [ ] Other (please explain)
Checklist:
- [x] If applicable, I have added corresponding documentation changes.
- [x] If applicable, I have reviewed the feature and / or service widget guidelines.
- [x] I have checked that all code style checks pass using pre-commit hooks and linting checks.
- [x] If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
Or even just display
Thanks. Agree customapi is a reasonable place for this.
one quibble is you use “vertical view” and “list view” interchangeably which is confusing. Let’s just call it “list”. And instead of a Boolean let’s use something like
displayType: list
I also find the currency and trend params potentially overly-specific (even your docs note the potential for confusion there). I need to look at that more closely, if they really belong here at all
@shamoon, thanks for your reply.
I will then have display
of list
and row
which default to row
.
Regarding the currency and trend parameters, since I am copying most of the code from the Coin Market Cap widget, I just kept them as they are, as they also fit my particular use case in mind. Also, I made them both optional, so it would not interfere with other usages.
Since I am using my own API, I can easily add currency symbols in my response, so I don't mind removing the currency parameter. Also, as specified in the documentation, it might be showing NaN when combined with data transformation if the value after transformation is not a number.
However, I do find the trend indicator very helpful; after all, it displays a line, and having a secondary value would be really helpful. I thought about having a second optional 'field', then it would be too tedious to have the value changing its color. So I just went with limiting the secondary field to a colored trend indicator.
Please share your thoughts on this, and I can change my code accordingly. Also, I appreciated that you agreed to have a list view for the custom API, looking forward to your reply.
display: list
and should default to block
, I think.
Yea, the issue with currency and trend is I dont see them being used in any other case other than the market list you have above (are there any?). It's mixing specificity. If this PR is about adding a list view, it should be a pure list view, which has nothing to do with currency nor trending. They should both be removed from here.
display: list
and should default toblock
, I think.Yea, the issue with currency and trend is I dont see them being used in any other case other than the market list you have above (are there any?). It's mixing specificity. If this PR is about adding a list view, it should be a pure list view, which has nothing to do with currency nor trending. They should both be removed from here.
Sounds good, I will update the code and docs.
However, can we have a secondary field for the list view, as there will be more space to display information? I can remove the coloring part and the percentage sign, just to keep an optional field as plain text for the potential use of additional information.
However, can we have a secondary field for the list view, as there will be more space to display information? I can remove the coloring part and the percentage sign, just to keep an optional field as plain text for the potential use of additional information.
Yea that could work, it just needs to be generic (and shouldn't be named 'trend' presumably), like somehow you can specify it as a secondary field but it can be any of the types used by the customapi widget. LMK if that makes sense
Yea that could work, it just needs to be generic (and shouldn't be named 'trend' presumably), like somehow you can specify it as a secondary field but it can be any of the types used by the customapi widget. LMK if that makes sense
Sounds good, I guess I will name it secondaryField
or additionalField
? I went through some docs for other widgets, this seems to be the naming convention used in this project.
I wonder if you find having another parameter to define the color of this information would be too much? It will be genetic and optional, and also it clarifies the boundary between the first field and the second field. I can have a list of options for colors to avoid any attack, and also add more customization to the widget.
So I am still trying the achieve a colored trend indicator more genetically.
Either one, additionalField
seems fine.
As for the color, that could be OK as its an optional parameter too (additionalFieldColor
or the like) but definitely should be short and sweet for now. Perhaps supporting a hex color and something like "positiveNegative" (maybe theres a better name for that) which would be a special option to color red / green below / above zero.
As for the color, that could be OK as its an optional parameter too (
additionalFieldColor
or the like) but definitely should be short and sweet for now. Perhaps supporting a hex color and something like "positiveNegative" (maybe theres a better name for that) which would be a special option to color red / green below / above zero.
Please correct me if I get it wrong, but a special option to color based on below/above zero(despite very much I would like to have), would first assume the additional field is not empty also it can compared to zero directly(with no trailing percentage sign).
In my option to keep the parameter simple and genetic, I can implement another function to be used when additionalField
is present. So the color will be determined using the function that could interpret the additionalFieldColor
parameter as face value or path to value in JSON. Also, it validates the value to be correct as a valid tailwind CSS text color(supporting tailwind colors, hex values, and/or opacity). In this way, the color could be static, or dynamically allocated specified by an API call. In this way, it eliminates the possibility of attack or misuse of the CSS class input.
There the end user can use static color code or specify a field in the API response that specifies the color code. So no assumption was made using the implementation and it is flexible to adapt to different use cases.
Yes, additionalFieldColor
would only be relevant if additionalField
is specified, it wouldn't be a problem if not, though.
I didnt follow every point of the middle of your last comment, might just be easier to see what you mean. I agree, the simpler the config the better, absolutely. Again, with regards to the API providing the color, are there any other examples of that besides the stock market API youre using? I bet not. It seems too-specific...
The only thing to note is that tailwind classes need to be already included statically, e.g. https://github.com/gethomepage/homepage/blob/ebd384c62dcff0e0ec4b27097f4b9baee83a0bdb/tailwind.config.js so its not as simple as allowing any class (and IMHO its not worth it to add a bunch of classes there just for this). I'd be OK with being able to choose from the ones that are already included (which also includes green and red even though they're not on that list) instead of allowing any HEX. Again, let's keep it simple.
I didnt follow every point of the middle of your last comment, might just be easier to see what you mean. I agree, the simpler the config the better, absolutely. Again, with regards to the API providing the color, are there any other examples of that besides the stock market API youre using? I bet not. It seems too-specific...
So firstly, no, there is no such API that provides this sort of color info, I run my own API server so supplying any sort of data does not bother me, this way it just proves more customization for those who want to control the color dynamically by API calls.
So I am thinking of just providing an additionalFieldColor
that only accepts string color and auto
which tries to infer the color by its value. I am thinking of using regular express to extract the first number in additionalField
and compare it to zero. This way it is rather simple and clear, also one could try to rely on auto to change the color accordingly.
The only thing to note is that tailwind classes need to be already included statically, e.g. https://github.com/gethomepage/homepage/blob/ebd384c62dcff0e0ec4b27097f4b9baee83a0bdb/tailwind.config.js so its not as simple as allowing any class (and IMHO its not worth it to add a bunch of classes there just for this). I'd be OK with being able to choose from the ones that are already included (which also includes green and red even though they're not on that list) instead of allowing any HEX. Again, let's keep it simple.
As for the second part, I think tailwind CSS supports all the text colors that showing in it docs by default, I can only provide support for those colors which does not need any additional classes to be added. I will try to implement it later and see if that works.
Sorry for the confusion, tailwind indeed purges all unused classes when creating the CSS file. Currently, the CSS file only includes about 20 text colors with various shades of color. presumedly is used by all different parts of the page.
There is no clear relation to what is included in the project already, I don't think there is an easy way to introduce a color selection without introducing more to the CSS file.
I am thinking of two solutions, there are 22 colors excluding white and black officially used by Tailwind, I can add 'text-color-300'(or another shade) to the CSS which only introduces 22 more classes which is relatively small when compared to the whole CSS file.
Or I am trying the find a way to reuse theme colors, as all of the colors are included in the CSS file already. However, I could not seem to be able to refer to the defined theme color for the text.
I just figured out a way to reuse the theme color!!!! I can just instead of specifying a 'text-color-shade', I can do 'theme-color text-theme-shade'. Then I effectively added no extra CSS classes, but allowing a wide selection of colors.
By redefining the theme color at a local level, I can reuse the color defined in the CSS file to color the text. I think this might be the best solution for now, indeed it adds some coupling for the colors of the theme and the color available for the text, but I can't think of a better solution without refactoring the whole color system.
Do let me know what you think about the solution, and then I can implement the code.
At this point I’m a bit lost in the weeds, I’d say just go ahead and do it, if you’re ok with that. Easier to talk about actual code than hypothetical
At this point I’m a bit lost in the weeds, I’d say just go ahead and do it, if you’re ok with that. Easier to talk about actual code than hypothetical
I have done the coding part, I will have you updated after some testing.
Here is an example of what I have implemented. There are four possibilities namely auto, black/white, color-shade, and some random value.
The auto option checks if there is a number in the value of additionalField
, if so, it will display the color accordingly.
The black/white option will just display the text in black/white.
The color-shade option will also change the color if and only if the color is in the available list of colors.
Lastly, any other value, and by default, the additionalField
will not have additional text color classes, so it will be displayed just like the Field
.
Here is the list of available colors that were added by the theme coloring.
The only problem is that it has all the colors, but not all the shades. Going by 100s there is 300 and 400 missing in the current CSS. Currently, I just set shades 300 and 400 as invalid, but it might be better to include them as it will just be two more CSS classes and it completes the selection for coloring.
I have the new implementation published, please let me know if there is anything else that you find needs changing. I will be checking the repository tomorrow. Thanks a lot for your feedback today, looking forward to contributing to the project soon.
I have made changes that address your concerns, please take a look and let me know if there is still anything missing.
- for auto, regex are slow, I think better to just try to parse as a number. I know you had it matching for any number in a string but I think most APIs will return a number alone when thats what its supposed to represent (and thus would be valuable for
auto
)
If you don't mind, I would like to add prefix
to the data transformation, as it would also be valuable to have customization on both sides.
Also, there are some issues with the reserved characters, for example, % is not allowed. But supplying the suffix in quotation marks works to solve this issue. I will update the documentation to reflect the preferred way to input suffixes in the examples.
Great, thanks for making all the changes. I think this is solid.
Thank you. I'm eagerly anticipating the next release. I'd like to mention that the renaming of the mapping
variable in the formatValue
function was done because the function was called twice: once with mapping
and once with mapping.additionalField
. I felt that keeping it named mapping
might be misleading, as it may not always refer to the mapping object.
Trying this, I don't seem to be able to get the additionalField parameter to displey:
widget:
type: customapi
url: http://10.0.10.9:9995/json?target=127.0.0.1:3493
refreshInterval: 60000 # In milliseconds, set to ~60s
method: GET
display: list
mappings:
- field:
ups:
nutcase.battery.runtime
label: Runtime
additionalField:
ups:
input.voltage
- field:
ups: nutcase.ups.load.watts
label: Power
additionalField:
ups:
nutcase.ups.status.composite
Commenting on merged PRs is not the best way to get help