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

feat: save 2 css snippets #459

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented May 13, 2024

Overview

Save snippet that only existed in CMS content. Migrate a note to an existing snippet.

Related

None.

Changes

  • added templates/css-global-a11y.html
  • added note to templates/css-banner-tacc.html

Testing

Note

Not required. This change merely saves code that has been working live.

  1. Open two different pages on https://tacc.utexas.edu website.
  2. Make window narrow enough that mobile nav appears.
  3. Verify mobile nav button has white-gray border.
  4. Open mobile nav.
  5. Focus on search field.
  6. Verify search field button has thick white border.

UI

sans new snippet with new snippet
before snippet after snippet

@wesleyboar wesleyboar changed the title feat: save css-homepage-2024-02 snippet feat: save 2 css snippets May 13, 2024
@wesleyboar wesleyboar marked this pull request as ready for review May 13, 2024 17:29
Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Notes for reviewers.

Comment on lines +8 to +12
/*
.nav-link:focus-visible,
#header-logo:focus-visible,
.navbar-dark .navbar-toggler:focus-visible {
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

I do not know why this is commented out. When I remember, or when prodded, I will comment it or remove it accordingly.

@@ -23,7 +26,7 @@
left: 0; top: 0; bottom: 0; right: 0;
background-color: var(--global-color-primary--x-light);
}
.banner-cell--major a:hover .u-highlight {
.banner-cell--major a:hover :is(.u-highlight, .highlight) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Two class names are supported: ITCSS class name, simple class name.

Comment on lines +1 to +3
<template id="css-banner-tacc--notes">
<!-- TO DYNAMICALL REDUCE HEIGHT OF BANNER: NO NEED to edit this stylesheet. Edit banner "Picture / Image" on CMS. Set `style="height: 60vh; max-height: 550px; min-height: 230px;"`. To use static height, set `style="height: 550px"` instead. -->
</template>
Copy link
Member Author

Choose a reason for hiding this comment

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

If dev is asked to change height of the banner, and sees this snippet, they may be tempted to work in here. But this stylesheet intentionally does not manage the banner height.¹

  1. Why? Core-CMS code clones style attribute given to a CMS image to appear on an <a> or <figure> tag that wraps the <image>. This hack² fixes a bug³ that Django CMS does not account for when it provides the feature that adds the wrapper. An <a> is added when the Picture / Image has Link settings. A <figure> is added when the Picture / Image is given a Caption.
  2. Introspection on the "hack" by @wesleyboar has led him to realize that Django CMS templates intentionally do not consider every case; they are samples that he has been treating as finished products. Their "bugs"³ may be his failure to replace the templates with ones that work as he demands.
  3. The bug is that the styles intended for the image box apply to the image without a wrapper and with a wrapper, which results in different rendering.

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.

None yet

1 participant