Skip to content

Conversation

@tconkling
Copy link
Contributor

@tconkling tconkling commented May 25, 2021

Updating PR from @hyerrakalva (#3176)

Fixes #3220


Original PR description:
Issue: #3071

Description: This PR fixes visual inconsistencies that may occur when creating widgets in a beta_columns container without specifying a label.

  • Updated Selectbox component's label+help element structure to make it vertically align properly with other widget types when all widgets in a beta_columns container are unlabeled
  • Updated Selectbox.test.tsx to ensure that a label element always exists within a Selectbox component, even if a label is not specified, for accessibility purposes
  • Added 2 more CSS properties to StyledWidgetLabel component to ensure that widgets in beta_columns container are vertically aligned when there is a combination of labeled & unlabeled widgets

hyerr1 and others added 3 commits April 25, 2021 20:12
Remove renderLabel method and update selectbox label stucture to use
that of similarly sized widgets like text_input
Update StyledWidgetLabel's CSS properties to ensure that a widget's
vertical position remains the same with or without a label
# By Ken McGrady (19) and others
# Via GitHub (9) and others
* develop: (96 commits)
  st.form: clear_on_submit (streamlit#3287)
  Init test variables in beforeEach
  Init test variables in beforeEach
  Add database tutorials to docs (streamlit#3272)
  update contact email to support (streamlit#3282)
  Better typing
  Remove dummy script
  Add testing and some comments
  Fix the bug, don't love the solution
  Improve test name
  Add script for testing
  Fix test after merge
  Make sure selected options aren't visible in the dropdown
  Widget test cleanup (streamlit#3286)
  Add test for the fuzzy filter
  Import filter function from another component
  Remove tag from PR template (streamlit#3284)
  Add test req version bound for TF to fix tests (streamlit#3266)
  pin click to < 8.0 (streamlit#3256)
  Up version to 0.82.0
  ...

# Conflicts:
#	frontend/src/components/shared/Dropdown/Selectbox.test.tsx
@tconkling tconkling force-pushed the fix-vertical-align branch from b1b6b56 to 6ae09c4 Compare May 25, 2021 00:26
@tconkling
Copy link
Contributor Author

@tvst is taking a look at the CSS changes today. If he's cool with them, I'll update the failing Cypress snapshots.

@tvst
Copy link
Contributor

tvst commented Jun 2, 2021

Found some vertical alignment issues which possibly predated this PR:
Screen Shot 2021-06-02 at 4 39 53 PM

So I fixed them using flex:
Screen Shot 2021-06-02 at 4 47 47 PM

See code here: 50b91a4
And this works with empty labels too.

tconkling added 4 commits June 7, 2021 13:26
* develop:
  Fix ReST vs Markdown typos and update conda docs link (streamlit#3371)
  update requirements file documentation (streamlit#3372)
  Fix a peculiar issue with yarn start (streamlit#3374)
  Remove unused mocha dependency (streamlit#3326)
  Update `typed-signals` to 2.2.0 (streamlit#3325)
  update classnames lib to 2.3.1 (streamlit#3331)
  Update DOMPurify to 2.2.8 (the latest) (streamlit#3327)
  Update protobufjs to 6.11.2 (streamlit#3329)
  Update react to latest point release (streamlit#3328)
  Update @craco/craco to 6.1.2 (streamlit#3324)
  Update testing-library to the latest (streamlit#3330)
  Update clipboardjs to 2.0.8 (streamlit#3332)
  Update JSON5 to 2.2.0 (streamlit#3333)
  * Add explanatory comment for naive email validation (streamlit#3349)
  ComponentInstance: cancel the componentReadyWarningTimer on unmount (streamlit#3316)
  Don't unmount iframe after custom component timeout (streamlit#3315)
…ical-align

* commit '50b91a4a18ce620f4e0e980f40f6f5b2503e1d33':
  Fix vertical alignment of tooltip icon
@tconkling tconkling marked this pull request as ready for review June 7, 2021 21:37
@tconkling tconkling requested a review from a team June 7, 2021 21:37
@tconkling
Copy link
Contributor Author

Thanks, Thiago! (I've merged @tvst's changes in here.)

@tconkling
Copy link
Contributor Author

This PR introduces some subtle changes to the way the ColorPicker border renders - @tvst, is this cool/intentional?

@kajarenc
Copy link
Collaborator

kajarenc commented Jun 8, 2021

@tconkling Probably out of the scope of this PR, but is there some reason why we don't take the same approach with

  {element.label}
  {element.help && (
    <StyledWidgetLabelHelp>
      <TooltipIcon
        content={element.help}
        placement={Placement.TOP_RIGHT}
      />
    </StyledWidgetLabelHelp>
  )}
</StyledWidgetLabel>

For Radio and ColorPicker components?

@tconkling tconkling merged commit 0cc9424 into streamlit:develop Jun 8, 2021
@tconkling tconkling deleted the fix-vertical-align branch June 8, 2021 20:42
@tconkling
Copy link
Contributor Author

@tconkling Probably out of the scope of this PR, but is there some reason why we don't take the same approach with

  {element.label}
  {element.help && (
    <StyledWidgetLabelHelp>
      <TooltipIcon
        content={element.help}
        placement={Placement.TOP_RIGHT}
      />
    </StyledWidgetLabelHelp>
  )}
</StyledWidgetLabel>

For Radio and ColorPicker components?

I don't think there's a good reason - just different people touching the files at different times. Might be worth a drive-by cleanup down the line.

tconkling added a commit to tconkling/streamlit that referenced this pull request Jun 9, 2021
* develop: (21 commits)
  config.py: types + cleanup (streamlit#3393)
  add faq item regarding use of PYTHONPATH (streamlit#3391)
  Fix vertical align (streamlit#3317)
  Create visualize_rent_prices_with_Streamlit.md (streamlit#3193)
  Update app resource limits (streamlit#3384)
  Bump dns-packet from 1.3.1 to 1.3.4 in /frontend (streamlit#3341)
  Bump ws from 6.2.1 to 6.2.2 in /frontend (streamlit#3358)
  Fix ReST vs Markdown typos and update conda docs link (streamlit#3371)
  update requirements file documentation (streamlit#3372)
  Fix a peculiar issue with yarn start (streamlit#3374)
  Remove unused mocha dependency (streamlit#3326)
  Update `typed-signals` to 2.2.0 (streamlit#3325)
  update classnames lib to 2.3.1 (streamlit#3331)
  Update DOMPurify to 2.2.8 (the latest) (streamlit#3327)
  Update protobufjs to 6.11.2 (streamlit#3329)
  Update react to latest point release (streamlit#3328)
  Update @craco/craco to 6.1.2 (streamlit#3324)
  Update testing-library to the latest (streamlit#3330)
  Update clipboardjs to 2.0.8 (streamlit#3332)
  Update JSON5 to 2.2.0 (streamlit#3333)
  ...
@tvst tvst mentioned this pull request Aug 2, 2021
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.

Alignment of widgets in a row if they are of different types is broken

4 participants