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: wiring ControlLabel to a new FormLabel #10388

Merged
merged 4 commits into from
Jul 23, 2020

Conversation

mistercrunch
Copy link
Member

Creating new simple component and wiring all
and react-bootstrap.ControlLabel towards it.

FormLabel becomes a pivotal point that can be altered to point to AntD
when we're ready.

Creating new simple <FormLabel /> component and wiring all <label>
and react-bootstrap.ControlLabel towards it.

FormLabel becomes a pivotal point that can be altered to point to AntD
when we're ready.
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020

Codecov Report

Merging #10388 into master will increase coverage by 0.03%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10388      +/-   ##
==========================================
+ Coverage   70.41%   70.45%   +0.03%     
==========================================
  Files         604      605       +1     
  Lines       32429    32430       +1     
  Branches     3298     3301       +3     
==========================================
+ Hits        22836    22848      +12     
+ Misses       9487     9476      -11     
  Partials      106      106              
Flag Coverage Δ
#cypress 55.48% <80.00%> (+0.14%) ⬆️
#javascript 59.35% <81.81%> (+0.02%) ⬆️
#python 69.77% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset-frontend/src/CRUD/Field.jsx 22.22% <ø> (ø)
...erset-frontend/src/SqlLab/components/SaveQuery.jsx 77.77% <ø> (ø)
...tend/src/SqlLab/components/ScheduleQueryButton.jsx 8.62% <ø> (ø)
superset-frontend/src/components/TableSelector.jsx 86.20% <ø> (ø)
...rc/dashboard/components/FilterIndicatorTooltip.jsx 100.00% <ø> (ø)
...ntend/src/dashboard/components/PropertiesModal.jsx 47.94% <ø> (ø)
...shboard/components/filterscope/FilterFieldItem.jsx 50.00% <ø> (ø)
.../src/explore/components/AdhocMetricEditPopover.jsx 81.48% <ø> (ø)
...-frontend/src/explore/components/ControlHeader.jsx 92.59% <ø> (ø)
...rontend/src/explore/components/EmbedCodeButton.jsx 70.00% <ø> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fdc141...8c83a0b. Read the comment docs.

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

LGTM with one small note.

Also note that form items in Antd normally would just accept label text as an attribute and does not need a Label component. Approving nevertheless for the improved consistency.

import React, { ReactNode } from 'react';
import { ControlLabel } from 'react-bootstrap';

interface Props {
Copy link
Member

Choose a reason for hiding this comment

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

FormLabelProps would be preferred.

@mistercrunch mistercrunch merged commit b438ba9 into apache:master Jul 23, 2020
@mistercrunch mistercrunch deleted the one-label branch July 23, 2020 07:27
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
* chore: wiring ControlLabel to a new FormLabel

Creating new simple <FormLabel /> component and wiring all <label>
and react-bootstrap.ControlLabel towards it.

FormLabel becomes a pivotal point that can be altered to point to AntD
when we're ready.

* lint

* ViewportControl

* addressing comments
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 0.38.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants