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(leaving-ibm): v2 style updates #10941

Conversation

sangeethababu9223
Copy link
Member

Related Ticket(s)

Closes #10856

Description

Style / layout break in Leaving IBM Component in V2

Changelog

Changed

  • button element changed
  • Styles updated to match the latest

@sangeethababu9223 sangeethababu9223 requested a review from a team as a code owner September 15, 2023 06:06
@sangeethababu9223 sangeethababu9223 linked an issue Sep 15, 2023 that may be closed by this pull request
1 task
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Sep 15, 2023

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Sep 18, 2023

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Sep 18, 2023

@oliviaflory
Copy link
Contributor

oliviaflory commented Sep 19, 2023

@sangeethababu9223 this is looking really good from a design perspective! A few fixes I found:

    • The focus state for the link is getting cut off when (tabbing into the link) see the screen shot attached
      Screenshot 2023-09-19 at 3 53 31 PM
    • I cannot interact with the link within the modal, was it like that in the v1 version?
    • There are no interactive states on the link within the modal (hover, focus, click) they should match the standard link states, you can see them documented in the Carbon link website docs or CWC
    • The focus should probably be trapped within the modal as it is within the v1 version, right now I can use the keyboard to tab outside of the modal and navigate around on the page under the overlay which will be confusing for the users.
    • On mobile the heading and close button are getting cut off at the top of the browser, while there is a gap at the bottom of the browser and I can see overlay. I am expecting to see the same behavior as the v1 version where the heading is not cut off and the button is pinned to the bottom and I should not see the overlay.
Current mobile Expecting
Screenshot 2023-09-19 at 4 06 01 PM Screenshot 2023-09-19 at 4 06 24 PM

Copy link
Contributor

@IgnacioBecerra IgnacioBecerra left a comment

Choose a reason for hiding this comment

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

It's looking good! Olivia addressed most of the things I wanted to mention, so just a few comments code-wise, will re-review once the design changes have been implemented :) 👍

@sangeethababu9223
Copy link
Member Author

@oliviaflory,
All of the issues except the focus trap not working in modal should be fixed now.
Focus trap seems to be an issue with Modal component. I've created a separate ticket for this: #10966
Will work on that once this issue is resolved.

Copy link
Contributor

@oliviaflory oliviaflory left a comment

Choose a reason for hiding this comment

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

@sangeethababu9223 Thank you! FYI the focus trap is working for me on Chrome, but it would be good to track for all browsers if you are noticing it not working on another 😄

@sangeethababu9223
Copy link
Member Author

Thanks @oliviaflory,
I faced issue with focus trap in in both Safari & Firefox. I'll look into this for the other ticket.

@kennylam kennylam merged commit 6e5aef5 into carbon-design-system:feat/carbon-for-ibm-dotcom-v2 Sep 26, 2023
4 of 14 checks passed
kennylam added a commit to kennylam/carbon-for-ibm-dotcom that referenced this pull request Jun 11, 2024
* feat(leaving-ibm): update to v2

---------

Co-authored-by: kennylam <[email protected]>
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.

[Component] Leaving IBM
5 participants