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

Search modal stuck open on page load with 5.0.10 includes but newer CSS #1349

Closed
ericras opened this issue Aug 16, 2019 · 4 comments
Closed

Comments

@ericras
Copy link
Member

ericras commented Aug 16, 2019

If a site is using the main template (https://github.com/unl/wdntemplates/blob/develop/Templates/fixed.dwt) then they have local html includes that may not sync until overnight but the html document fetches core.css from unlcms.unl.edu. So it may be that the version of the html includes doesn't match the css/js. (In the future this needs to be eliminated, but that will be for 5.1 or 6.0: #1348)

This was eliminated in 5.0.11 that is needed for a site that still has 5.0.10 includes:
digitalcampusframework/dcf@5902364#diff-13ba1e99069ebd6d875c7b3ceafc2c19

Possible solutions (or ways to think about this):

  1. DCF needs to be better versioned (or versioned period) and not remove things until a major version change - (can be moved to a deprecated file in minor releases). A major version change would then be a really big red flag for downstream themes to test an update to DCF.
  2. DCF needs to be better versioned and removing things is fine - but it needs a CHANGELOG file that helps document changes to be aware of upon updating.
  3. Even with (2), UNL would still have a problem. This isn't DCF's fault and is UNL's fault for doing a local/cloud template (An individual template needs to be all-cloud or all-local #1348). UNL's theme adds digitalcampusframework/dcf@5902364#diff-13ba1e99069ebd6d875c7b3ceafc2c19 in some sort of css "bridge" file that gets included in core then removed with the next release.

Search modal classes:

5.0.10
page load
dcf-modal-overlay dcf-mobile-toolbar-modal dcf-bg-overlay-light dcf-p-0
open
dcf-modal-overlay dcf-mobile-toolbar-modal dcf-bg-overlay-light dcf-p-0 dcf-modal-open

5.0.13 
page load
dcf-modal-overlay dcf-bg-overlay-light dcf-p-0 dcf-invisible unl-search-modal
open
dcf-modal-overlay dcf-bg-overlay-light dcf-p-0 unl-search-modal
css in 5.0.10
.dcf-modal-overlay[aria-hidden="true"] {
    opacity: 0;
    pointer-events: none;
    -webkit-transition: opacity .4s ease-out,visibility 0ms .4s;
    transition: opacity .4s ease-out,visibility 0ms .4s;
    visibility: hidden;
}

css in 5.0.13  
.unl-search-modal[aria-hidden="true"] {
    opacity: 0;
    pointer-events: none;
    -webkit-transition: opacity .4s ease-out;
    transition: opacity .4s ease-out;
}
@ericras
Copy link
Member Author

ericras commented Aug 16, 2019

This really seems to be a mismatched includes & css (local & cloud/cdn combined) problem.

I don't have an explanation at this time for why a local implementation like @cmdprompt has would have had the problem.

@skoolbus39
Copy link
Member

I agree we need to version DCF.

The reason for the changes to the modal was to fix the broken modals for those using prefers-reduced-motion. In retrospect, I probably should have used the existing DCF class instead of changing it to a theme class. I'll work on getting the styles present for both to account for mismatched include files and CSS.

@ericras
Copy link
Member Author

ericras commented Oct 21, 2019

Do we need a transition css file (that is built into core) in which
.dcf-modal-overlay[aria-hidden="true"] {
opacity: 0;
pointer-events: none;
-webkit-transition: opacity .4s ease-out,visibility 0ms .4s;
transition: opacity .4s ease-out,visibility 0ms .4s;
visibility: hidden;
}

can be included then removed later (next release)?

EDIT: I guess its more complicated than just that

@ericras
Copy link
Member Author

ericras commented Oct 24, 2019

#1359 1359

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

No branches or pull requests

2 participants