Skip to content
This repository has been archived by the owner on Jan 27, 2025. It is now read-only.

fix: show focus border for scrolling tabs #992

Merged
merged 3 commits into from
Nov 2, 2021

Conversation

MizuhoOkimoto
Copy link
Contributor

@MizuhoOkimoto MizuhoOkimoto commented Oct 30, 2021

Please check if the PR fulfills these requirements

  • The commit message follows semantic commit message guidelines
  • The changes are documented in component docs and changelog
  • The ESLint plugin has been updated if a new component is added
  • Test have been added or modified, if appropriate
  • Has been verified locally

What kind of change does this PR introduce?
Closes #987
Please see this page: https://atomic-react.security.cisco.com/components/tab#scrolling
When clicking a tab, the top border is missing.
I added padding-top 1px, so that the top border became visible.

Does this PR introduce a breaking change?
No

Other information:
Attached screenshot after fixing it.
image

If I need to improve or follow any rules to contribute, please let me know. Thank you very much!

@frattaro frattaro changed the title fix: scrolling tabs (#987) fix: show focus border for scrolling tabs Nov 1, 2021
Copy link
Contributor

@frattaro frattaro left a comment

Choose a reason for hiding this comment

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

Can you add an entry in /framework/changelog.mdx? as

## 3.1.3 (pending)

- Fixed padding on scrolling tabs so focus border displays correctly

@MizuhoOkimoto
Copy link
Contributor Author

MizuhoOkimoto commented Nov 1, 2021

Hi @frattaro, I added the changelog inside the above file. I appreciate it if you can review it. Thank you!

@frattaro
Copy link
Contributor

frattaro commented Nov 1, 2021

@MizuhoOkimoto Thanks, did it get pushed?

@MizuhoOkimoto
Copy link
Contributor Author

Hi, @frattaro ! Yes, I pushed it. Was it okay to push...?
image
image

@frattaro
Copy link
Contributor

frattaro commented Nov 1, 2021

I mean the changelog edits aren't showing up in this PR -- still only 2 commits.

I'm not sure why it's not prompting me to approve the CI workflows for this PR either.

If you can figure it out that's cool, if not I'd suggest abandoning the fork and cloning the repo and branching

@MizuhoOkimoto
Copy link
Contributor Author

Hi @frattaro , I tried many ways to push my change(add: changelog 3.1.3), and I used force-push as a last resort. I usually don't want to use force/hard, but it was the last attempt before cloning the repo again.
I believe I pushed it and updated the snapshot. Could you please review it?

@frattaro
Copy link
Contributor

frattaro commented Nov 2, 2021

Can you pull in the latest? I added pull_request_target to the CI workflow, maybe it'll trigger for this PR that way

@frattaro frattaro dismissed their stale review November 2, 2021 18:32

Resolved

@frattaro
Copy link
Contributor

frattaro commented Nov 2, 2021

I'm going to merge this -- but it's strange that the CI workflow isn't running. I assume it's because it's from a fork. Can you please clone/branch next time?

@frattaro frattaro merged commit 67ecebc into cisco-sbg-ui:master Nov 2, 2021
@MizuhoOkimoto
Copy link
Contributor Author

Hi @frattaro , thank you so much. Sorry for taking your time to check CI and this problem. I will make another fork and clone/branch next time!

@frattaro
Copy link
Contributor

frattaro commented Nov 2, 2021

Thank you too, the contributions are much appreciated -- sorry for the road bumps :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrolling tabs focused tab display issue
2 participants