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.

Sign up for GitHub

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

Jump to bottom

Update behavior and view when adding new note from trash #2618

Merged
merged 1 commit into from
Feb 25, 2021
Merged

Update behavior and view when adding new note from trash #2618

merged 1 commit into from
Feb 25, 2021

Conversation

sandymcfadden
Copy link
Contributor

@sandymcfadden sandymcfadden commented Feb 1, 2021 •
edited
Loading

Fix

Currently when you are viewing the trash adding a new note is confusing.
This PR resolves #2612 by updating the view to all notes when adding a note from trash.

It also resolves #2627 by updating the views which appear when there are no notes in a list.

no-tags
no-search-results
empty-trash
empty-note-list

Test

  1. Visit trash screen.
  2. Notice New Note icon appears disabled.
  3. Clicking New Note icon does not create new note.
  4. Pressing keyboard shortcut Cmd or Ctrl + Shift + i does create a new note
  5. After note is created you have the all notes view.
  6. Remove all notes from trash.
  7. Notice there is no link to create a new note.

Additional testing

  1. Visit All Notes
  2. Trash all notes
  3. Notice updated empty note list view
  4. Search for a non existent term
  5. Notice updated empty note list view
  6. Visit a tag which have no notes
  7. Notice updated empty note list view

Release

  • Update the messaging and view when no notes show in a specific list.
  • Fixes the view when adding a new note when viewing the trash.

@sandymcfadden sandymcfadden added this to the 2.7.0 milestone Feb 1, 2021
@sandymcfadden sandymcfadden requested a review from a team February 1, 2021 19:10
@sandymcfadden sandymcfadden self-assigned this Feb 1, 2021
dmsnell
dmsnell reviewed Feb 1, 2021
View reviewed changes
lib/menu-bar/index.tsx Outdated Show resolved Hide resolved
dmsnell
dmsnell reviewed Feb 4, 2021
View reviewed changes
lib/note-list/no-notes.tsx Outdated
const getButton = () => {
return searchQuery.length ? (
const getSearchButton = () => {
return (
<button
onClick={() =>
dispatch(actions.ui.createNote({ content: searchQuery }))
Copy link
Member

Choose a reason for hiding this comment

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

🤨 nothing like deep coupling - how'd that get in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I'm not sure if my approach to this refactoring of the empty note list views is the best way to go. Any thoughts would be much appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

best policy is to not change code styling in an unrelated PR so I wouldn't recommend doing anything differently. that being said I occasionally point out things in code review that are unrelated but which stand out to me. this is one of those because I saw dispatch inside a UI component, which is something that sets off all the alarms in my head

@sandymcfadden sandymcfadden changed the title Disable the ability to add a new note when in the trash Update behavior and view when adding new note from trash Feb 4, 2021
sandymcfadden
sandymcfadden commented Feb 4, 2021
View reviewed changes
lib/search/index.ts Show resolved Hide resolved
lib/note-list/no-notes.tsx Outdated
const getButton = () => {
return searchQuery.length ? (
const getSearchButton = () => {
return (
<button
onClick={() =>
dispatch(actions.ui.createNote({ content: searchQuery }))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I'm not sure if my approach to this refactoring of the empty note list views is the best way to go. Any thoughts would be much appreciated.

@sandymcfadden sandymcfadden mentioned this pull request Feb 5, 2021
Closed
@sandymcfadden sandymcfadden modified the milestones: 2.7.0, 2.8.0 Feb 5, 2021
@sandymcfadden sandymcfadden requested review from dmsnell and a team February 10, 2021 17:26
dmsnell
dmsnell reviewed Feb 11, 2021
View reviewed changes
lib/note-list/no-notes.tsx Outdated Show resolved Hide resolved
codebykat
codebykat requested changes Feb 24, 2021
View reviewed changes
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.

This looks good and works well!

One small request. Similar to iOS, I think the "Create your first note" text should be a link that creates a note.

@sandymcfadden
Copy link
Contributor Author

Good catch, thanks @codebykat. I followed the designs, but that makes sense to have it as a link. I have it updated now.

codebykat
codebykat approved these changes Feb 24, 2021
View reviewed changes
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.

Works great 👍

dmsnell
dmsnell reviewed Feb 25, 2021
View reviewed changes
lib/note-list/no-notes.tsx Outdated Show resolved Hide resolved
@sandymcfadden sandymcfadden force-pushed the update/disallow-creating-note-from-trash branch from d619c07 to d8ad44a Compare February 25, 2021 11:39
@sandymcfadden
Disable the ability to add a new note when in the trash
51cbb72 
Move new note button title back into jsx.

Reenabling keyboard shortcut while in trash and refactoring the no notes views

Update styling for empty note list views

Updating the approach of setting the empty note screens based on feedback.

Update create first note to be a link

Moving type declaration to module global scope

Restore package-lock.json file
@sandymcfadden sandymcfadden force-pushed the update/disallow-creating-note-from-trash branch from d8ad44a to 51cbb72 Compare February 25, 2021 12:18
@sandymcfadden sandymcfadden modified the milestones: 2.8.0, 2.9.0 Feb 25, 2021
@sandymcfadden sandymcfadden merged commit 3473a34 into develop Feb 25, 2021
@sandymcfadden sandymcfadden deleted the update/disallow-creating-note-from-trash branch February 25, 2021 15:02
@pachlava pachlava mentioned this pull request Mar 11, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmsnell dmsnell dmsnell left review comments

@codebykat codebykat codebykat approved these changes

Assignees

@sandymcfadden sandymcfadden

Labels
None yet
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.

UI Spring cleaning: Update empty note list screens Disallow adding new note when in trash.
3 participants
@sandymcfadden @codebykat @dmsnell