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.

Sign up for GitHub

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

Jump to bottom

[web-components]: Masthead has some styling issues #11251

Closed
2 tasks done
NicoletaBordea opened this issue Dec 19, 2023 · 1 comment · Fixed by #11252
Closed
2 tasks done

[web-components]: Masthead has some styling issues #11251

NicoletaBordea opened this issue Dec 19, 2023 · 1 comment · Fixed by #11252
Assignees
@jkaeser
Labels
bug Something isn't working dev Needs some dev work Masthead L0/L1 v2 used when component or pattern is part of the Mashead V2 update effort

Comments

@NicoletaBordea
Copy link

NicoletaBordea commented Dec 19, 2023 •
edited by andy-blum
Loading

JIRA

Description

I noticed two issues on masthead styling:

  1. navigation item is not highlighted for current page, even if active attribute is present, it doesn't have the blue border on bottom (I am using with custom navigation items)

  2. profile menu is missing some styles, dropdown looks ugly

Component(s) impacted

masthead

Browser

Chrome

Carbon for IBM.com version

v2.0.0-rc.3

Severity

Severity 2 = Aspects of design is broken, and impedes users in a significant way, but there is a way to complete their tasks. Affects major functionality, has a workaround.

Application/website

ibmdocs

Package

@carbon/ibmdotcom-web-components

CodeSandbox example

https://codepen.io/jakaeser/pen/gOqErGE

Steps to reproduce the issue (if applicable)

No response

Release date (if applicable)

No response

Code of Conduct
@NicoletaBordea NicoletaBordea added bug Something isn't working dev Needs some dev work labels Dec 19, 2023
@jkaeser jkaeser added the Masthead L0/L1 v2 used when component or pattern is part of the Mashead V2 update effort label Dec 19, 2023
@jkaeser jkaeser self-assigned this Dec 19, 2023
@jkaeser jkaeser mentioned this issue Dec 19, 2023
Merged
@NicoletaBordea
Copy link
Author

I found one more small styling issue. Not sure if this is a bug or not. The Platform name (Cloud in this example) has smaller font than the navigation links, and looks little bit odd:
https://carbon-design-system.github.io/carbon-for-ibm-dotcom/next/web-components/?path=/story/components-masthead--with-cloud-data

@kodiakhq kodiakhq bot closed this as completed in #11252 Jan 11, 2024
kodiakhq bot pushed a commit that referenced this issue Jan 11, 2024
@jkaeser
feat(masthead): spacing style regression fix and maintainability refa…
67cc49b 
…ctor (#11252)

## Related Ticket(s)

Resolves #11251 

## Description

This PR mostly refactors masthead source code to be more maintainable and removes dead code. Other than the style fixes, it should result in no visible changes to the masthead.

## Changelog

### Styles
#### Changed
- Makes necessary functions available to Sass stylesheets to fix style regressions.

#### Removed
- Removes Cloud-specific masthead styles.
---
### Services Store
#### Changed
- Deprecates `MastheadProfileContent` type.
- Deprecates `Translation.mastheadNav` in favor of `Translation.masthead.nav`.
- Deprecates `Translation.profileMenu` in favor of `Translation.masthead.profileMenu`.

#### Removed
- Removes references to v1 data structures that are no longer in use.
---
### IBM.com Web Components
#### Changed
- Deprecates the `navLinks` Masthead Composite property in favor of the more descriptive `l0Data` property.
- Splits out the Masthead Composite's main `render` method into more digestible render methods per logical section.
- Untangles the knot that was the `renderNavItems` method by providing dedicated `renderTopNav` and `renderLeftNav` methods for the desktop and mobile experiences.
- Extracts distinct chunks of logic into their own methods for readability and in some cases reuse across the newly split render methods.

#### Removed
- Removes Cloud-specific masthead.
- Removes non-functional `customNavLinks` property. Use `l0Data` or `l1Data` instead.
- Removes masthead v1's leftover CTA functionality.
kennylam pushed a commit to kennylam/carbon-for-ibm-dotcom that referenced this issue Mar 21, 2024
@jkaeser @kennylam
feat(masthead): spacing style regression fix and maintainability refa…
f781c88 
…ctor (carbon-design-system#11252)

Resolves carbon-design-system#11251

This PR mostly refactors masthead source code to be more maintainable and removes dead code. Other than the style fixes, it should result in no visible changes to the masthead.

- Makes necessary functions available to Sass stylesheets to fix style regressions.

- Removes Cloud-specific masthead styles.
---
- Deprecates `MastheadProfileContent` type.
- Deprecates `Translation.mastheadNav` in favor of `Translation.masthead.nav`.
- Deprecates `Translation.profileMenu` in favor of `Translation.masthead.profileMenu`.

- Removes references to v1 data structures that are no longer in use.
---
- Deprecates the `navLinks` Masthead Composite property in favor of the more descriptive `l0Data` property.
- Splits out the Masthead Composite's main `render` method into more digestible render methods per logical section.
- Untangles the knot that was the `renderNavItems` method by providing dedicated `renderTopNav` and `renderLeftNav` methods for the desktop and mobile experiences.
- Extracts distinct chunks of logic into their own methods for readability and in some cases reuse across the newly split render methods.

- Removes Cloud-specific masthead.
- Removes non-functional `customNavLinks` property. Use `l0Data` or `l1Data` instead.
- Removes masthead v1's leftover CTA functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jkaeser jkaeser

Labels
bug Something isn't working dev Needs some dev work Masthead L0/L1 v2 used when component or pattern is part of the Mashead V2 update effort
Projects
Carbon for IBM.com
Archived in project
Milestone
No milestone
2 participants
@jkaeser @NicoletaBordea