-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Sub columns for services #1662
Sub columns for services #1662
Conversation
Sorry for my formater 🙄 |
This breaks widgets (in a non-graceful way) in the sub columns, which I think most people would expect to keep working. |
Indeed didn't saw it, i will have a look. |
Yea this is why I would discuss a change like this before PR. How did you plan to handle service widget layouts with these extremely narrow columns? |
I was thinking that the user could revert to a less restrictive layout if this one doesn't fit. |
I will probably also need to tweak the sub-columns breakpoints to be more aggressive. |
This feels overly-complicated for a feature that no one seems to have requested... Not really up to me tho |
Just my two cents, I would use this. |
Nice ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small thing is your linter makes this PR hard to look at clearly.
But bigger picture, I appreciate you've put a lot of effort into this but Im not really in favor of merging it. It feels like a lot of work to shoehorn this feature in, it seems to make the code overall less understandable and probably difficult to debug (e.g. https://github.com/benphelps/homepage/pull/1662/files#diff-706cde307938b4f6c07b100f1e1a27af392451424f1cc25ca7c15f8150ed1b5dR86 ) and again, even if there are some people who would use this, I dont think this is a particularly-desired feature and I dont love how service widgets are handled.
Again, not sure its really up to me.
I understand, |
I could maybe see a use for it, but I think its a niche feature. My use case would be to list my ESXi hosts under the vCenter management console:
I could live without it though |
Again, really appreciate the work OP but I think we're going to leave this out for now. Mostly it feels kinda niche for something that requires so many changes (and results in significantly less readable code imho). In the future perhaps open a discussion before putting so much work into something just so it doesnt end up going un-merged =( Thanks again |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
Proposed change
Add the ability to have sub-columns services.
The key number indicate the number of sub-columns.
Type of change
Checklist:
pnpm lint
.