Skip to content

WEB-159: Update "base" label to "phone" - #144

Open
Quaaaaaaaaaark wants to merge 5 commits into
mainfrom
WEB-159
Open

Quaaaaaaaaaark wants to merge 5 commits into
mainfrom
WEB-159

Conversation

@Quaaaaaaaaaark

Copy link
Copy Markdown
Collaborator

Updated the "base" label to "phone" in responsive styling menus to be more clear for users. Additionally, renamed the "base" ResponsiveBreakpoint variable to "phone" within the code to be more clear for development. Changed "base" to "phone" in documentation as well where relevant.

@Quaaaaaaaaaark
Quaaaaaaaaaark requested a review from leec18 October 3, 2026 20:51
@Quaaaaaaaaaark Quaaaaaaaaaark self-assigned this Oct 3, 2026
@vercel

vercel Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
website-development Ready Ready Preview Oct 11, 2026 5:13pm UTC

@leec18 leec18 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

besides the missing changes this lgtm!

Comment thread docs/STYLING.md
{ base: "sm" } // same at all sizes
{ base: "sm", md: "md", lg: "lg" } // changes at each breakpoint
{ base: "sm", lg: "lg" } // skip md — it inherits from base
{ phone: "sm" } // same at all sizes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is me nitpicking but i suggest changing "phone" key to 'sm' to match the other two keys

I'll leave this up to you to change or not !

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On second thought, it might be better for you to leave it as it is since we have a isPhone boolean variable too... what are your thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Even before I swapped base and phone it was 'base' rather than 'sm' with an isBase variable (plus I think changing it here would require it to be changed in a lot of other contexts too) so my thought is to leave it as it is... let me know if you decide you do want me to change it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i agree with you, let's leave it for now but i will bring it up with Grace n Shlok cuz you do raise a good point !

Comment thread docs/STYLING.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this comment should be updated as well

Comment thread docs/STYLING.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

change base to phone

Comment thread docs/STYLING.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here too

(sorry for the all comments... ik its hard looking through everything n its easy to miss things so dont worry)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Wow I really thought I caught all of the "base" instances... I'll get right on that.

Comment thread docs/STYLING.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here

This branch was successfully deployed

1 active deployment
Preview — a27d9940 Deployed Oct 11, 2026 by vercel[bot]
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