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

When there are no notes suggest creating one #2422

Merged
merged 11 commits into from
Oct 30, 2020
Merged

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Oct 26, 2020

Fix

When a user does not have any notes suggest adding a new note. Additionally, if a user types a search term that does not match any notes suggest creating a new not with that query.

This also fixes a bug where if you have no notes "Notes Loading" neve disappears.

Screen Shot 2020-10-26 at 4 58 14 PM

Screen Shot 2020-10-26 at 4 59 17 PM

Screen Shot 2020-10-26 at 4 58 42 PM

Screen Shot 2020-10-26 at 4 58 57 PM

Test

  1. Have no notes, observe note list suggests you add a new note
  2. Have a search query that does not match any notes. Observe note list suggest that you create one with that search term

Release

Suggest creating a new note when none exists or notes match search query.

@belcherj belcherj added this to the 2.1.0 milestone Oct 26, 2020
@belcherj belcherj self-assigned this Oct 26, 2020
lib/note-list/no-notes.tsx Outdated Show resolved Hide resolved
@belcherj belcherj requested review from iamthomasbishop and a team October 26, 2020 21:12
@iamthomasbishop
Copy link

Looks pretty good at first glance @belcherj, I like this type of suggestion/tip on search. I'm not 100% familiar with y'all's color palette in light vs. dark mode, so I'll leave that to @SylvesterWilmott , but the messaging and placement makes sense to me. A couple of suggestions — both very minor, subjective, and to be taken with a grain of salt:

  • Feel free to overrule me if there's an established rule/guideline here, but the text for the "create a new note" button feels rather large to me. Perhaps a "secondary" label size that's just a tad smaller would feel less so?

  • I also think it'd benefit from a slight bit of margin between it and the "no results" label. I'm not sure what spacing/grid you're using if any, but just a small unit of margin — something like 8px of margin if on an 8px grid — would probably be enough.

Obviously very minor things, and if established rules should negate either, feel free to bypass 😄

@belcherj belcherj marked this pull request as ready for review October 28, 2020 19:36
@belcherj
Copy link
Contributor Author

@iamthomasbishop I made the text smaller and added padding between the button and the text.

@iamthomasbishop
Copy link

@belcherj nice, thanks!

Is it just my eyes, or is the blue color being used for the icons in the top toolbar different from the button text below in dark mode? Light mode seems about right but it's hard to tell for sure. Screenshot:

Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, tests good. 👍

We definitely need to focus the text within the editor after creating the new note, so you can create a new note and immediately continue typing.

Also, should we consider clearing the search query after the new note is created? Right now you end up here, which is kind of a lot of highlighting:

Screen Shot 2020-10-28 at 9 21 00 PM

@belcherj
Copy link
Contributor Author

We are now focusing the editor when changing notes and creating a new note. (1281ee9)

#2434 will clear the search when a new note is created.

@codebykat codebykat self-requested a review October 30, 2020 02:48
@SylvesterWilmott
Copy link
Contributor

Feel free to overrule me if there's an established rule/guideline here, but the text for the "create a new note" button feels rather large to me. Perhaps a "secondary" label size that's just a tad smaller would feel less so?

That is correct, the label size here should 14px based on the design found in Zeplin. @belcherj can we make sure we update this?

@belcherj
Copy link
Contributor Author

Done

@belcherj belcherj merged commit b526468 into develop Oct 30, 2020
@belcherj belcherj deleted the add/no-notes-create-note branch October 30, 2020 13:40
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.

4 participants