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

"Show less common lifecycles" shows more lifecycles - confusing wording #2

Closed
wants to merge 1 commit into from

Conversation

danvc
Copy link

@danvc danvc commented Apr 6, 2018

Hey brother, how are you doing?

Nice work on this diagram! I'm just creating this PR to fix a small issue. Have a good weekend!

The current implementation doesn't apply the "Show Less" logic according the current value.

The current implementation doesn't apply the "Show Less" logic according the current value.
@wojtekmaj
Copy link
Owner

Hey Dan! Thanks for your words and for PR.

The checkbox is actually working correctly, but wording should be better, because I see it confuses some people, including you.

"Show less common lifecycles" means "Show me also those lifecycles which are less common" and NOT "Show me less lifecycles".

Any ideas to make it more clear?

@wojtekmaj wojtekmaj changed the title Fixes as issue related to the checkbox "Show less common lifecycles" shows more lifecycles - confusing wording Apr 7, 2018
@danvc
Copy link
Author

danvc commented Apr 7, 2018

Hi Wojciech!

You're right. Changing the label will would help. How about: "Show all lifecycles"? Once the "basic" lifecycles are already being shown, once selected would show all cycles.

After reading your source code, looks like that it's very structured. I learned about Parcel, which is awesome, and also about localStorage (which I've been using IndexedDB instead).

Congrats!

@wojtekmaj
Copy link
Owner

Hmmm, I'll give it a thought ;) Let me convert this PR into an issue.

Regarding the source code - that was actually my biggest complaint about this code :D I coded it very fast and not thinking about it a lot. Now I ordered it and it's a lot better ;)

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.

None yet

2 participants