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

Allow collapsing details of online hosts #8

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Allow collapsing details of online hosts #8

merged 2 commits into from
Feb 11, 2020

Conversation

Anthirian
Copy link
Contributor

I've expanded the template a bit by adding support for collapsing the details of online hosts upon clicking the header bar.

@honze-net honze-net self-assigned this Feb 4, 2020
@honze-net honze-net added the enhancement New feature or request label Feb 4, 2020
@honze-net
Copy link
Owner

Hi @Anthirian,
this is a great idea. I tried it and works! But the user has no visual indication, that this is collapsible. Do you have an idea on how to improve that?
Thanks
Andreas

@Anthirian
Copy link
Contributor Author

Thanks. I don’t unfortunately, but I’ll look into it soon.

@honze-net
Copy link
Owner

No problem. I'll have a look, too.

@Anthirian
Copy link
Contributor Author

I think I figured it out, please have a look at the latest version.

@honze-net
Copy link
Owner

honze-net commented Feb 10, 2020

This looks great. I have a few notes:
One would be the whitespace between the chevron and the IP address. The space is quite small, I would increase it by a bit, like an n or m wide space.
The second is, that the chevron states are flipped around. They should point right if closed and down if opened. If you click too fast, you can create a race condition, where the chevron will change, but the container will not collapse or the other way round. So it can become inconsistent.

I will have a look and try to come up with a solution for that.

@Anthirian
Copy link
Contributor Author

Yeah I noticed that as well, but I wasn't sure what was causing it.

@honze-net honze-net changed the base branch from master to test February 11, 2020 10:10
@honze-net
Copy link
Owner

I found a way to replace the js toggle of glyphicons with css, that reads the state of the container. So no race condition anymore. The glyphicons switched places and I added some padding. I have merged it into a test branch and I will apply all changes there.

@honze-net honze-net merged commit 441e993 into honze-net:test Feb 11, 2020
honze-net added a commit that referenced this pull request Feb 11, 2020
@honze-net
Copy link
Owner

Please test my latest commit in the test branch. If it’s okay for you, I will merge this into master after your approval.

@honze-net honze-net added the good first issue Good for newcomers label Feb 11, 2020
@Anthirian
Copy link
Contributor Author

Looks good to me, nice fix 👍

@honze-net
Copy link
Owner

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants