[MG_A1_01]: Add header layout - #2
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe placeholder page is replaced with a content-driven responsive landing page. New Sass tokens, mixins, components, themes, responsive header styles, IcoMoon assets, project metadata, and runtime navigation/menu behavior are added. ChangesLanding page implementation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant indexjs as src/script/index.js
participant Content as public/content.json
participant DOM
Browser->>indexjs: Load front-end script
indexjs->>Content: Fetch page content
Content-->>indexjs: Return navigation and button labels
indexjs->>DOM: Render links and buttons
Browser->>DOM: Click hamburger menu
DOM->>indexjs: Trigger click handler
indexjs->>DOM: Toggle classes and ARIA attributes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d9b2377-4be0-4ae6-903f-faebb77a5b01
⛔ Files ignored due to path filters (23)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.jsonpublic/assets/fonts/icomoon.eotis excluded by!**/*.eotpublic/assets/fonts/icomoon.svgis excluded by!**/*.svgpublic/assets/fonts/icomoon.ttfis excluded by!**/*.ttfpublic/assets/fonts/icomoon.woffis excluded by!**/*.woffpublic/assets/images/Ellipse 23.pngis excluded by!**/*.pngpublic/assets/images/Group 3.svgis excluded by!**/*.svgpublic/assets/images/airbnb.pngis excluded by!**/*.pngpublic/assets/images/booking.pngis excluded by!**/*.pngpublic/assets/images/carousel.pngis excluded by!**/*.pngpublic/assets/images/city.jpgis excluded by!**/*.jpgpublic/assets/images/expedia.pngis excluded by!**/*.pngpublic/assets/images/globe.pngis excluded by!**/*.pngpublic/assets/images/logo.svgis excluded by!**/*.svgpublic/assets/images/mountain.jpgis excluded by!**/*.jpgpublic/assets/images/object.svgis excluded by!**/*.svgpublic/assets/images/ocean.jpgis excluded by!**/*.jpgpublic/assets/images/orbitz.pngis excluded by!**/*.pngpublic/assets/images/star.svgis excluded by!**/*.svgpublic/assets/images/travel-point.pngis excluded by!**/*.pngpublic/assets/images/triangles.pngis excluded by!**/*.pngpublic/assets/images/tripadvisor.pngis excluded by!**/*.pngpublic/assets/images/user.jpgis excluded by!**/*.jpg
📒 Files selected for processing (28)
.coderabbit.yaml.github/workflows/pr-check.ymlindex.htmlpackage.jsonpublic/assets/fonts/selection.jsonpublic/content.jsonpublic/styles/main.csssrc/script/index.jssrc/styles/main.scsssrc/styles/scss/abstract/_breakpoint.scsssrc/styles/scss/abstract/_color.scsssrc/styles/scss/abstract/_function.scsssrc/styles/scss/abstract/_index.scsssrc/styles/scss/abstract/_mixin.scsssrc/styles/scss/base/_index.scsssrc/styles/scss/base/_reset.scsssrc/styles/scss/base/_typography.scsssrc/styles/scss/components/_button.scsssrc/styles/scss/components/_cards.scsssrc/styles/scss/components/_floater.scsssrc/styles/scss/components/_icon.scsssrc/styles/scss/components/_index.scsssrc/styles/scss/layouts/_header.scsssrc/styles/scss/layouts/_index.scsssrc/styles/scss/themes/_index.scsssrc/styles/scss/themes/_light.scsssrc/styles/scss/vendors/_icomoon.scsssrc/styles/scss/vendors/_index.scss
| - name: Checkout (full history for commit range) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Harden the checkout step’s credentials and action pinning.
actions/checkout@v4 is not pinned to an immutable commit, and credential persistence is enabled. Pin the action to a full commit SHA and disable persisted credentials so later steps cannot unintentionally reuse the repository token.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 15-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
Source: Linters/SAST tools
| { | ||
| "name": "static-html-base-template", | ||
| "description": "", | ||
| "description": "### Prerequisites", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the misleading package description.
"### Prerequisites" is a Markdown heading, not a description of this project, so package metadata consumers will see incorrect information.
| //create flex with default values which are commonly used throughout project | ||
| @mixin flex( | ||
| $direction: row, | ||
| $space: space-between, | ||
| $gap: 1rem, | ||
| $align-item: center | ||
| ) { | ||
| display: flex; | ||
| flex-direction: $direction; | ||
| justify-content: $space; | ||
| gap: $gap; | ||
| align-items: $align-item; | ||
| } | ||
|
|
||
| //responding to different screen sizes as defined in breakpoint.scss | ||
| @mixin responsiveUp($screen) { | ||
| @media (min-width: map.get(bp.$breakpoints,$screen)) { | ||
| @content; | ||
| } | ||
| } | ||
|
|
||
| //adding shadow | ||
| @mixin shadow( | ||
| $x: 0, | ||
| $y: 62px, | ||
| $blur: 17px, | ||
| $color: color.change(cl.$black-800, $alpha: 0.1), | ||
| $rad: 0 | ||
| ) { | ||
| box-shadow: $x $y $blur $rad $color; | ||
| } | ||
|
|
||
| //changing font properties | ||
| @mixin font($size, $color, $weight) { | ||
| color: $color; | ||
| font-size: $size; | ||
| font-weight: $weight; | ||
| } | ||
|
|
||
| //creating grid layout | ||
| @mixin grid($col, $gap: 1rem) { | ||
| display: grid; | ||
| grid-template-columns: repeat($col, 1fr); | ||
| gap: $gap; | ||
| } | ||
|
|
||
| //to apply typography properties associated to semantic names | ||
| @mixin useTypo($typography-type) { | ||
| @each $key, $value in $typography-type { | ||
| #{$key}: $value; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add standard Sass documentation for reusable mixins.
The current comments do not document parameters, defaults, or expected inputs. This is especially unclear for responsiveUp’s breakpoint key and useTypo’s map structure.
As per path instructions, standard Sass documentation/comments are required for mixins.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 21-21: Expected mixin name "responsiveUp" to be kebab-case (scss/at-mixin-pattern)
(scss/at-mixin-pattern)
[error] 53-53: Expected mixin name "useTypo" to be kebab-case (scss/at-mixin-pattern)
(scss/at-mixin-pattern)
Source: Path instructions
dbcc7f7 to
1f43655
Compare
| border-radius: f.px-to-rem(100); | ||
| padding: 0.5rem 1rem; | ||
| max-width: f.px-to-rem(115); | ||
| @include mix.use-typo(typo.$nav-link-2); |
| @use '../base/typography' as typo; | ||
|
|
||
| //defining styling for parent header container | ||
| .container { |
There was a problem hiding this comment.
according to BEM it should have been header
| @include mix.responsive-up(md) { | ||
| .container { | ||
| flex-direction: row; | ||
| //button inside header container | ||
| &__btn { | ||
| max-width: f.px-to-rem(220); | ||
| @include mix.flex($gap: f.px-to-rem(8)); | ||
| flex-wrap: wrap; | ||
| } | ||
| //the navigation link box that pop up when hamburger icon is clicked | ||
| &__nav-box { | ||
| width: 90%; | ||
| top: f.px-to-rem(75); | ||
| left: 7%; | ||
| padding-right: 0.75rem; | ||
| @include mix.flex($gap: 1.5rem); | ||
| flex-wrap: wrap; | ||
| } | ||
| //buttons inside navBox remain hidden in tablet or higher view | ||
| &__nav-box-btn { | ||
| display: none; | ||
| } | ||
| } | ||
| } |
| <div class="orangeHue"> | ||
| <img src="/assets/images/Ellipse 23.webp" alt="Coloured hue" /> | ||
| </div> |
1f43655 to
7489af3
Compare
2 participants
Summary by CodeRabbit