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

HTML and CSS cleaning #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

HTML and CSS cleaning #17

wants to merge 1 commit into from

Conversation

MikeFernandez-Pro
Copy link
Collaborator

I moved scripts into a separated folder "scripts", same for "styles" for CSS styles and restructured the HTML :

  • Added header, main, footer,
  • Placed div into sections with an h2 title

Copy link
Owner

@GuillaumeDua GuillaumeDua left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution,
there are a few minor changes requested as review comment down below to make it great

Once accepted, we might reuse some of those file not only for divs_html_elements test but also others.

You might wanna move some folders :

  • styles/ so it becomes tests/styles/
  • scripts/awesome-doc-configuration.js so it becomes `tests/scripts/awesome-doc_default-configuration.js

and then, for reusability purpose, refactorhttps://github.com/GuillaumeDua/awesome-doc-code-sections/blob/ce4493eadb3d899e209f550a2acf8fb424d51f59/tests/per_usage/custom_html_elements/index.html so it also gets benefits from it.

@@ -0,0 +1,26 @@
let is_stylished = false;
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather name that file "style-mutators" rather than "styling", so the purpose is explicit

}

let is_small = false;
function toggle_small() {
Copy link
Owner

Choose a reason for hiding this comment

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

toggle_small -> toggle_small_style

Right now, this function only works if toggle_style was called before.

An improvement here would be to add a simple if-then here, such as :

if (!is_stylished)
   toggle_style()

and modify toggle_style so when L7 it removes the classes, then it also reset sizes

L7 :

function (elements) {
        elements.removeClass(`${stylished_classname}`);
        if (is_small)
            toggle_small()
      }

@GuillaumeDua GuillaumeDua added the PR/issue: outdated PR or issue that requires an update first, prior to any action label Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/issue: outdated PR or issue that requires an update first, prior to any action
Projects
Status: Pending (requires evaluation)
Development

Successfully merging this pull request may close these issues.

3 participants