Skip to content

Core design updates #6817

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.

Sign up for GitHub

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

Jump to bottom
Merged
lukasmasuch merged 16 commits into from
Jun 16, 2023
Merged

Core design updates #6817

lukasmasuch merged 16 commits into from
Jun 16, 2023

Conversation

@sfc-gh-jgarcia
Copy link
Contributor

@sfc-gh-jgarcia sfc-gh-jgarcia commented Jun 8, 2023 •
edited
Loading

Describe your changes

This PR introduces a set of visual changes to our widgets:

  • Updates their border-radius by making them more rounded;
  • Makes st.checkbox and st.radio borders thinner;
  • Tightens up the margin-bottom between labels and widgets so they're closer;
  • Improve the way chevrons for st.selectbox and st.multiselect look like
  • Improve vertical alignment for st.checkbox's label

Issue link

https://www.notion.so/snowflake-corp/Open-Source-Refresh-part-1-c8b3e52d6d744fa6aabbaaafb3dd21fa?pvs=4

Testing Plan

  • Explanation of why no additional tests are needed: There are already tests in place for these widgets
  • Unit Tests (JS and/or Python): Updated JS tests
  • E2E Tests: Regenerated snapshots (sorry they're so many!)
  • Any manual testing needed? Not necessary, but feel free to play with it!

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-jgarcia sfc-gh-jgarcia changed the title Core refresh Core design updates Jun 8, 2023
@sfc-gh-jgarcia sfc-gh-jgarcia added security-assessment-completed Security assessment has been completed for PR impact:users PR changes affect end users change:other PR contains other type of change labels Jun 8, 2023
@sfc-gh-jgarcia sfc-gh-jgarcia marked this pull request as ready for review June 8, 2023 20:01
@sfc-gh-wihuang
Copy link
Contributor

sfc-gh-wihuang commented Jun 13, 2023

@sfc-gh-jgarcia , can you do a merge with develop for this PR?

sfc-gh-wihuang
sfc-gh-wihuang reviewed Jun 13, 2023
View reviewed changes
frontend/src/lib/components/widgets/Multiselect/Multiselect.tsx Outdated
overrides: {
Svg: {
style: () => ({
width: "24px",
Copy link
Contributor

@sfc-gh-wihuang sfc-gh-wihuang Jun 13, 2023

Choose a reason for hiding this comment

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

nit: is it possible to create a variable in sizes.ts that is like svgHeight: "24px" or "1.5rem"?

Or could we use iconSizes.xl?

Copy link
Contributor Author

@sfc-gh-jgarcia sfc-gh-jgarcia Jun 14, 2023

Choose a reason for hiding this comment

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

That's a great callout! Fixed on 34a0b0e

sfc-gh-wihuang
sfc-gh-wihuang reviewed Jun 13, 2023
View reviewed changes
frontend/src/lib/components/shared/Dropdown/Selectbox.tsx Outdated
overrides: {
Svg: {
style: () => ({
width: "24px",
Copy link
Contributor

@sfc-gh-wihuang sfc-gh-wihuang Jun 13, 2023

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

@sfc-gh-jgarcia sfc-gh-jgarcia Jun 14, 2023

Choose a reason for hiding this comment

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

Fixed on 34a0b0e

@sfc-gh-jgarcia
Copy link
Contributor Author

sfc-gh-jgarcia commented Jun 14, 2023 •
edited
Loading

@sfc-gh-jgarcia , can you do a merge with develop for this PR?

@sfc-gh-wihuang Done!

sfc-gh-wihuang
sfc-gh-wihuang approved these changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@sfc-gh-wihuang sfc-gh-wihuang left a comment

Choose a reason for hiding this comment

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

Honestly, I didn't check all of the screenshots but I checked some of them and they seemed reasonable. Code wise LGTM.

I would ask that you check all the screenshots for sanity sake :)

@sfc-gh-jgarcia
Copy link
Contributor Author

sfc-gh-jgarcia commented Jun 15, 2023 •
edited
Loading

Honestly, I didn't check all of the screenshots but I checked some of them and they seemed reasonable. Code wise LGTM.

I would ask that you check all the screenshots for sanity sake :)

@sfc-gh-wihuang Yeah, I don't blame you, haha. I did check them, and they look good to me, so we should be good 👍

@lukasmasuch lukasmasuch merged commit bba53cd into develop Jun 16, 2023
@sfc-gh-jgarcia sfc-gh-jgarcia mentioned this pull request Jul 3, 2023
Merged
kajarenc pushed a commit that referenced this pull request Jul 3, 2023
@sfc-gh-jgarcia
Add missing border radius (#6944)
ebc5848 
This PR is a follow up of #6817, updating the border-radius from .25rem to .5rem to the following widgets:

- st.code;
- st.expander;
- st.info, st.warning, st.success and st.error;
- st.exception
@sfc-gh-kmcgrady sfc-gh-kmcgrady deleted the core-refresh branch October 5, 2023 19:31
asmeralt pushed a commit to asmeralt/streamlit that referenced this pull request Sep 29, 2025
@sfc-gh-jgarcia @asmeralt
Add missing border radius (streamlit#6944)
11027aa 
This PR is a follow up of streamlit#6817, updating the border-radius from .25rem to .5rem to the following widgets:

- st.code;
- st.expander;
- st.info, st.warning, st.success and st.error;
- st.exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sfc-gh-wihuang sfc-gh-wihuang sfc-gh-wihuang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

change:other PR contains other type of change impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sfc-gh-jgarcia @sfc-gh-wihuang @lukasmasuch