Skip to content

NTNGL-5467 | Side to Side Arrow Buttons#7064

Open
NicholasHallman wants to merge 16 commits into
mainfrom
NTNGL-5467/flippable-arrow-buttons
Open

NTNGL-5467 | Side to Side Arrow Buttons#7064
NicholasHallman wants to merge 16 commits into
mainfrom
NTNGL-5467/flippable-arrow-buttons

Conversation

@NicholasHallman

Copy link
Copy Markdown
Contributor

For list tiles, we want to change the directional movement arrows to left and right from up and down.

Functional Testing

  • LTR: "Up" is left, "Down" is right
  • RTL: "Up" is right, "Down" is left
  • new vdiffs

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL https://live.d2l.dev/prs/BrightspaceUI/core/pr-7064/

Note

The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@NicholasHallman NicholasHallman marked this pull request as ready for review June 11, 2026 15:22
@NicholasHallman NicholasHallman requested a review from a team as a code owner June 11, 2026 15:22
@NicholasHallman NicholasHallman requested a review from dbatiste June 11, 2026 15:22
Comment thread components/button/button-move.js Outdated
Comment thread components/button/button-move.js Outdated
:host([disabled-down]) .down-layer {
cursor: default;
}
:host([side-to-side]:not(:dir(rtl))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did anything need to be done with the origin to maintain the visual (0,0) position of the element?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suspect I'll need to do some moving around for the list's themselves, everything rotated just fine here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... looks like we probably need to do something here. Besides the left edge, it also means that elements on the right-hand side would also be too close. I'm wondering if adjusting switching to a horizontal flex layout would be better than rotating in-place. Though the one-liner is nice.

image

@NicholasHallman NicholasHallman Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think you're right... Okay I'll start on that good call

Comment thread components/button/demo/button-move.html Outdated
Comment thread components/button/demo/button-move.html Outdated
.up-layer,
.down-layer {
height: 1.2rem;
.layer {

@NicholasHallman NicholasHallman Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've put the up and down layers into a new layer div which allows me to easily change the flex direction while maintining the slightly larger size of the absolutly positioned buttons

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we probably need to make space for the overall width of the clickable targets. This screenshot shows how the clickable targets overflow their container. I know this happens in the vertical layout too, but we've accepted it there because the expected contexts all provide space, and we didn't want to change their respective layouts. In the side-to-side case though, I'm more worried, because I think it has greater chance of overlapping with adjacent click targets.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah good call, should be an easy tweak

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@NicholasHallman NicholasHallman requested a review from dbatiste June 12, 2026 18:42

:host([side-to-side]) d2l-icon {
/* Arrows need to be rotated in opposite direction for RTL */
transform: rotate(calc(-90deg * var(--d2l-length-factor)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not opposed to the rotation, and I noticed that we do not have arrow-toggle-left and arrow-toggle-right icons, but if we preferred actual icons, we can create them. We'd define them with mirror-in-rtl to automatically flip like we do the rest of our RTL sensitive icons. I didn't mention this before because the whole thing was being rotated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm open to making new icons for this, my reason for doing this is because it's a pretty nice css only solution. In RTL we need to flip the icons around because the flex box changes the rendering order.

In LRT it's: [<][>]
And in RTL it's [>][<]

So we'd either need templating logic to flip the arrow direction, or css rotations. Using the left and right arrows here is also harmless because they don't have an aria description so you'd never be given an inaccurate description. It's not revealed information in the accessibility tree.

@NicholasHallman

Copy link
Copy Markdown
Contributor Author
button with interactables button hit points with interactables

Hitboxes with the new changes. I can also bump the buttons and the hit zones down as well?

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants