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

Add state for empty screens #276

Closed
1 of 2 tasks
vbuberen opened this issue Mar 17, 2020 · 4 comments · Fixed by #325
Closed
1 of 2 tasks

Add state for empty screens #276

vbuberen opened this issue Mar 17, 2020 · 4 comments · Fixed by #325
Assignees

Comments

@vbuberen
Copy link
Collaborator

⚠️ Is your feature request related to a problem? Please describe

Currently Chucker shows no difference between a request with empty body and a request which somehow failed to fetch info to display. States should be distinct, so user could understand what is going on.

💡 Describe the solution you'd like

In cases where the request/response is really empty we should show some state telling user that there is an empty request/response.

📄 Additional context

Here is how empty state looks now
Screenshot 2020-03-17 at 22 00 16

🙋 Do you want to develop this feature yourself?

  • Yes
  • No
@vbuberen vbuberen added the enhancement New feature or improvement to the library label Mar 17, 2020
@vbuberen vbuberen self-assigned this Mar 17, 2020
@cortinico
Copy link
Member

Totally agree. Ideally having some graphic would be helpful. Maybe some Vector Drawable to do not impact on the library size. Otherwise just text would work fine.

@vbuberen
Copy link
Collaborator Author

Yes, I thought about drawable as well. Actually, I already picked one. Here is an example
Screenshot 2020-03-18 at 13 53 26
I will change the text in future, just tries to see how it looks. I selected such emoticon with neutral face, because empty payload isn't something bad. It is just a neutral state 😄

The problem with implementation of such state is in fact that with the current logic in TransactionPayloadFragment's AsyncTask we are transaction items into a TransactionPayloadItem and, thus, by the time we need to change visibility of items we already have just items with strings and can't clearly define if there is an empty request or response. So we need to update this logic. But, since we already have #270 opened I would prefer to merge it first and do updates there to not do the job twice. Also, it seems that the idea shared in #259 about splitting Request and Response into its own entities might be a good helper here as well.

@cortinico
Copy link
Member

Yes, I thought about drawable as well. Actually, I already picked one. Here is an example

I'd go for something more positive if possible (like this https://materialdesignicons.com/icon/package-variant).

I personally considered the neutral face to inform that something hasn't really worked as expected but that's no big deal (like a failure that is not preventing the app from working). Also please note that the majority of the HTTP requests (GETs) are without request body, so that icon will be shown a lot of times.

But that's just my personal taste.

Agree to let #270 and #259 go over this (this is a nice to have imho).

@vbuberen
Copy link
Collaborator Author

Your option is great actually. Overlooked an empty package icon when tried to pick something for this purpose.

@ghost ghost added the Pending PR The resolution for the issue is in PR label Apr 15, 2020
@ghost ghost removed Pending PR The resolution for the issue is in PR enhancement New feature or improvement to the library labels Apr 16, 2020
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 a pull request may close this issue.

2 participants