Repository navigation
WEB-159: Update "base" label to "phone" - #144
Quaaaaaaaaaark wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
leec18
left a comment
There was a problem hiding this comment.
besides the missing changes this lgtm!
| { 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 |
There was a problem hiding this comment.
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 !
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 !
There was a problem hiding this comment.
this comment should be updated as well
There was a problem hiding this comment.
here too
(sorry for the all comments... ik its hard looking through everything n its easy to miss things so dont worry)
There was a problem hiding this comment.
Wow I really thought I caught all of the "base" instances... I'll get right on that.
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.