Skip to content

Comments

Do not flash loading class in modal refresh#2982

Closed
mvorisek wants to merge 2 commits intofomantic:developfrom
mvorisek:remove_loading_class_remove_in_observer
Closed

Do not flash loading class in modal refresh#2982
mvorisek wants to merge 2 commits intofomantic:developfrom
mvorisek:remove_loading_class_remove_in_observer

Conversation

@mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Jan 19, 2024

introduced in Semantic-Org/Semantic-UI#5578

Is the hack using loading class still needed? It causes a problem in out environment because it flashes the class causing our observers to trigger.

Maybe more changes/LoC removed are needed/wanted.

@prudho
Copy link
Contributor

prudho commented Jan 30, 2024

Remove the loading hack looks ok to me, but deleting the CSS is not a good idea IMHO. Some users might want to make their modals in a loading state, when using ajax for example.

@mvorisek mvorisek force-pushed the remove_loading_class_remove_in_observer branch from 2f62015 to 57e6d87 Compare March 16, 2025 00:26
@mvorisek mvorisek marked this pull request as draft March 16, 2025 00:26
@mvorisek
Copy link
Contributor Author

mvorisek commented Mar 16, 2025

I still do not understand the Semantic-Org/Semantic-UI@7450aa3 described in Semantic-Org/Semantic-UI#5578 (comment). Maybe flashing the loading class is needed.

I am closing this PR as I am unsure about this PR, we needed also the CSS removed (https://github.com/atk4/ui/blob/37a5a0b522/js/src/JqueryPlugin/ReloadViewPlugin.js#L53) and we worked around it on atk4 side.

@mvorisek mvorisek closed this Mar 16, 2025
@mvorisek mvorisek deleted the remove_loading_class_remove_in_observer branch March 16, 2025 00:32
@lubber-de
Copy link
Member

lubber-de commented Mar 16, 2025

Jack was basically right: To be able to calculate/ access dimensions of DOM nodes, they have to be visible (to the browser, off canvas is fine, but display:none is not, as this does not occupy any visible space)
However, i still want #2969 to be implemented and perhaps the whole JS calculation isnt necessary anymore for that purpose, especially after the IE drop. I'll tag this for investigation

@lubber-de lubber-de added the state/awaiting-investigation Anything which needs more investigation label Mar 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/awaiting-investigation Anything which needs more investigation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants