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

Undo snackbar #222

Merged
merged 17 commits into from
Mar 24, 2021
Merged

Undo snackbar #222

merged 17 commits into from
Mar 24, 2021

Conversation

andOrlando
Copy link
Collaborator

@andOrlando andOrlando commented Dec 20, 2020

Closes #171
Adds dynamic snackbar support and an undo snackbar that pops up whenever you delete an assignment
snackbars being basically notification messages

image

public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/buttonFunctions.js Outdated Show resolved Hide resolved
public/js/buttonFunctions.js Outdated Show resolved Hide resolved
public/js/buttonFunctions.js Outdated Show resolved Hide resolved
public/js/buttonFunctions.js Outdated Show resolved Hide resolved
@notrodes
Copy link
Collaborator

I think this is a great canadate for using webcomponants to compartementalize the compleity of this addition

@psvenk
Copy link
Member

psvenk commented Dec 20, 2020

I wonder if we could make it more intuitive to undo deleting an assignment by not actually deleting the assignment's row from the table — we could grey out the row, exclude it from calculations, and replace the "x" button with a button to restore the assignment. The snackbar code could still be useful otherwise. @andOrlando Your thoughts?

public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
psvenk
psvenk previously requested changes Dec 21, 2020
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
public/js/home.js Outdated Show resolved Hide resolved
@notrodes notrodes self-requested a review December 22, 2020 22:21
@kdk1616
Copy link
Collaborator

kdk1616 commented Dec 23, 2020

Add removed assignments to a stack and then they can be returned with undo button (or Cmd+z)

@psvenk psvenk added this to the 2.7.0 milestone Dec 26, 2020
Copy link
Collaborator

@notrodes notrodes left a comment

Choose a reason for hiding this comment

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

split off into file

@andOrlando andOrlando dismissed stale reviews from notrodes and psvenk January 12, 2021 21:53

I put it in a different file

@psvenk psvenk modified the milestones: 2.6.2, 2.7.0 Jan 25, 2021
@psvenk psvenk modified the milestones: 2.7.0, 2.7.1 Feb 17, 2021
@andOrlando andOrlando force-pushed the undo-snackbar branch 2 times, most recently from 7546937 to 610e377 Compare March 17, 2021 13:50
Copy link
Member

@psvenk psvenk left a comment

Choose a reason for hiding this comment

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

Looks good overall. A few comments:

  • Deleting an assignment or clicking "undo" in the snackbar scrolls up to the top of the page
  • Clicking anywhere on the snackbar other than the "undo" button dismisses the snackbar permanently
  • Make Ctrl+Z more discoverable and make it use Command+Z on macOS

And some style nitpicks:

  • There are many overlong lines that should be broken up so that they are (for the most part) less than 80 characters long
    • I know that some people question the value of having a rigid limit, but some of these lines do many things at once and are more difficult to read, especially for people who use 80-column windows for editing code (we still exist!)
  • Comments should have a space between the comment delimiter and the text (// Comment)
  • Some statements are missing semicolons

andOrlando and others added 7 commits March 24, 2021 17:40
adds the snackbar class
- adds colors for undo snackbar (to be edited)
- improves resize logic for snackbar at low screensizes
- When assignment is deleted, replaces item with a placeholder that's ignored by tabulator which gets deleted if the snackbar times out
- adds a bunch of semicolons
- better document snackbar options
- adds timeout and funciton on timeout
- improves instant .show() logic
- improves oncick things
- fixes lots of other bugs
- makes placeholder deletion actually delete from the list
- also adds some semicolons
adds some semicolons, deletes test function
Co-authored-by: psvenk <45520974+psvenk@users.noreply.github.com>
Co-authored-by: psvenk <45520974+psvenk@users.noreply.github.com>
andOrlando and others added 10 commits March 24, 2021 17:40
- fixes bug where the timeout wouldn't run the correct function on timeout
- fixes bug where the timeout would start when the snackbar is made
- adds timeout modes for what to do when the timer finishes
- gives new assignments sequential IDs
- filters the assignments when determing whether to show the info icon or not
- changes right padding for button
I don't know how to get rid of this suggestion otherwise so I'll cave

Co-authored-by: psvenk <45520974+psvenk@users.noreply.github.com>
-fixes a random tooltips bug for some reason
-changes the ::after to ::before for no reason
-adds an undo-registry to keep track of all the undos
-adds states to make it easier to ascertain the status of a given snackbar
-slightly improves a couple comments
I left them in for debugging...
Rejoice! After a whole month or something I finally fixed that one bug!
(changed method of keeping track of snackbar stuff)
I was using unshift instead of shift which wasn't great, the new order
feels much more intuitive
Keep lines to <=80 characters, simplify some code, fix indentation
@psvenk psvenk merged commit e8eeefa into Aspine:master Mar 24, 2021
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.

When you delete an assignment it's just gone
4 participants