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

chore: Rename Breakout to Breakout Spotlight #1629

Closed
wants to merge 3 commits into from

Conversation

VincentSmedinga
Copy link
Contributor

@VincentSmedinga VincentSmedinga commented Oct 3, 2024

The name ‘Breakout’ is nice and short but not very descriptive of what it is and does. I considered ‘Spotlight With Media’ first, but that’s overly long, and we use ‘figure’ instead of ‘media’ in the docs and API.

I think ‘Spotlight Grid’ is a good option. It mentions the two main components playing a role in the composition instead of introducing a new term. It indicates being an extension of Grid. It also works well with the Cell subcomponent.

This is not a breaking change, as Breakout hasn’t been released yet.

RubenSibon
RubenSibon previously approved these changes Oct 4, 2024
Copy link
Contributor

@RubenSibon RubenSibon left a comment

Choose a reason for hiding this comment

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

I guess I could get behind the reasoning for this name change, but... The Spotlight part is spot on for sure, but I have some doubts about the Grid part. Using a Grid is one solution, but other routes are an option (such as the negative margins solution). And to what extent are developers interacting with a (CSS) Grid. It seems their options are rather limited.

But I can't think of many better names than this though... Maybe "Spotlight Breakout"?

@VincentSmedinga
Copy link
Contributor Author

I have some doubts about the Grid part. Using a Grid is one solution, but other routes are an option (such as the negative margins solution).

But we’ve chosen the Grid solution, and I don’t think we need to fear a change in implementation and a resulting rename.

And to what extent are developers interacting with a (CSS) Grid. It seems their options are rather limited.

They implement the responsive layout using the colStart, colSpan, rowStart, and rowSpan props of Breakout.Cell.

But I can't think of many better names than this though... Maybe "Spotlight Breakout"?

Well, that’s not bad at all. Although I’d swap the words to ‘Breakout Spotlight’, like we do with e.g. our input names.

@alimpens
Copy link
Contributor

I have some doubts about the Grid part. Using a Grid is one solution, but other routes are an option (such as the negative margins solution). And to what extent are developers interacting with a (CSS) Grid. It seems their options are rather limited.

I agree with this, the Grid part is an implementation detail imo, a user doesn't have to know about that.

Well, that’s not bad at all. Although I’d swap the words to ‘Breakout Spotlight’, like we do with e.g. our input names.

I like 'Breakout Spotlight'!

@VincentSmedinga
Copy link
Contributor Author

After performing the renaming, it does appear to introduce some weirdness.

‘Breakout Spotlight’ sounds like a spotlight itself, but then it contains a Spotlight.

<BreakoutSpotlight>
  <BreakoutSpotlight.Cell  has="spotlight" >
    <Spotlight />
  </BreakoutSpotlight.Cell></BreakoutSpotlight>

Is that something we can live with?

@github-actions github-actions bot temporarily deployed to demo-rename-breakout October 10, 2024 12:51 Destroyed
@VincentSmedinga VincentSmedinga changed the title chore: Rename Breakout to Spotlight Grid chore: Rename Breakout to Breakout Spotlight Oct 10, 2024
@VincentSmedinga
Copy link
Contributor Author

We decided to stick with Breakout after all – none of the alternatives are really better.

@VincentSmedinga VincentSmedinga deleted the chore/rename-breakout branch October 11, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants