Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

delete btn popup attached #684

Merged

Conversation

himanish-star
Copy link
Contributor

@himanish-star himanish-star commented Dec 23, 2017

fixes: #217

As requested by @pdehaan and @SoftVision-CiprianMuresan , I have changed the background color of delete btn to red and attached a popup message . Although I believe the hovering effects are not that great. If you suggest me then I'll change it accordingly

@dannycoates If you don't mind could you review my PR

Below is a Preview Video

ezgif com-video-to-gif 1

@himanish-star himanish-star changed the title background color of delete btn changed to red background color of delete btn changed to red and popup attached Dec 23, 2017
Copy link
Contributor

@dannycoates dannycoates left a comment

Choose a reason for hiding this comment

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

The popup looks fine, but please leave the colors unchanged.

@himanish-star
Copy link
Contributor Author

himanish-star commented Jan 6, 2018

@dannycoates , I've made the changes but there is one small problem.

This error shows up when I delete a file.

Uncaught TypeError: Cannot read property 'classList' of null

ezgif com-video-to-gif 2

This occurs as the function showPopup() gets called when we delete a file and thus the below line causes the problem.

popupText.classList.add('show');

I'm trying to fix this and will fix it soon.

@dannycoates dannycoates changed the title background color of delete btn changed to red and popup attached delete btn popup attached Jan 8, 2018
@dannycoates
Copy link
Contributor

I'd suggest changing popupText to something like:

const popupText = document.querySelector('.popuptext')

@himanish-star
Copy link
Contributor Author

himanish-star commented Jan 8, 2018

On it
EDIT : I have made the changes and the errors no longer persist. Thank you so much

@dannycoates dannycoates merged commit 069f0e5 into mozilla:master Jan 8, 2018
@dannycoates
Copy link
Contributor

Thanks @himanish-star 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete button does not bring up a confirmation dialog
2 participants