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

Hotfix to allow for modal to be scrolled #5486

Merged
merged 3 commits into from May 2, 2018
Merged

Conversation

spa49
Copy link
Contributor

@spa49 spa49 commented Apr 24, 2018

modal´s scrollbar was hidden by modal-backdrop hence could not be scrolled

according to this

@rmehta
Copy link
Member

rmehta commented Apr 25, 2018

@spa49 can you please add an animated GIF for before and after, so we understand the impact of your fix?

https://github.com/frappe/erpnext/wiki/Contribution-Guidelines

@@ -467,7 +467,7 @@ fieldset[disabled] .form-control {
}
.modal-backdrop {
opacity: 0.5;
position: fixed;
position: absolute;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please edit the desk.less file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @netchampfaris
I also changed the .less file since only changing less file and not .css file dint do anything
Is this right?
I´m not familiar with .less but if it is for dynamic styling why didnt it do anything after bench build and clear website cache?
I thought less is either to build a .css or to be used standalone... however according to chrome console the desk.css is the one controling the item modal-backdrop.
please clarify

@spa49
Copy link
Contributor Author

spa49 commented Apr 25, 2018

@rmehta
Sorry, actually didnt read the CGL to be honnest - now I did.

My personal protocoll is documentation with .gif etc only from 10+ lines changed or in general for severe programatic changes ... everything alse I just visualize Q n D plus Im so bold not to take style sheets serious enough - @netchampfaris has proven me wrong there as well ;-)
However my appologies.

so again....
If you want to send an email from document view you go via "Menu" - "Email" into the Modal.
If said Modal´s content exceeds the hight of the screen one would wish to scroll - however scrolling is prevented as scroll bar can not be selected via mouse/cursor. this is caused by the modal-backdrop item having a fixed position ( firefox clearly shows that the scroll bar is hidden behind the modal-backdrop, also not selectable , chrome however displays scroll bar in front of modal-backdrop but does not allow to be scrolled. If scoll bar is clicked/selected the modal closes
please see before.gif in chrome

email modal not scrollable

with the in PR proposed changes the scroll bar is usable
please see after.gif in chrome

email modal now scrollable

hope that clears it up... will remember in the future:-)

@netchampfaris netchampfaris merged commit 526ebb7 into frappe:hotfix May 2, 2018
saurabh6790 added a commit that referenced this pull request May 2, 2018
shridarpatil pushed a commit to zerodha/frappe that referenced this pull request May 29, 2018
* Update desk.css

* Update desk.less
shridarpatil pushed a commit to zerodha/frappe that referenced this pull request May 29, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2023
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.

None yet

3 participants