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

feat(bootstrap-4): support additionalProperties #2429

Merged

Conversation

nickgros
Copy link
Contributor

Reasons for making this change

Hey, so I saw #2229 hasn't been touched in a few months and it seemed like it wouldn't take much to get it over the finish line. I noticed that some of the work done in that PR was made irrelevant by #2405, so I've tried to remove the redundant code so it plays nicely.

This would partially fix #1927

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@danielstoa
Copy link

danielstoa commented Jun 18, 2021 via email

@epicfaace
Copy link
Member

@danielstoa
Copy link

danielstoa commented Jun 18, 2021 via email

@epicfaace
Copy link
Member

I can't, but you can contact github support at https://support.github.com/contact and they can get this resolved for you.

@abemedia
Copy link
Contributor

abemedia commented Jul 9, 2021

Hey @epicfaace, any chance of getting this merged?

@epicfaace
Copy link
Member

@epicfaace todo review this

@martinsch80
Copy link

Hey @epicfaace, any chance of getting this merged?

@epicfaace
Copy link
Member

Can you add it to the meeting agenda so we can schedule a synchronous review #2677

@florianf
Copy link

Added it to the meeting minutes, would be great if this could be merged.

icon="remove"
tabIndex={-1}
disabled={disabled || readonly}
onClick={onDropPropertyClick(label)}
Copy link
Member

Choose a reason for hiding this comment

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

make this button red

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added variant="danger"

/>
</Form.Group>
</Col>
<Col xs={6}>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Col xs={6}>
<Col xs={5}>

<Col xs={6}>
{children}
</Col>
<Col xs={1} className="py-4">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Col xs={1} className="py-4">
<Col xs={2} className="py-4">

</Col>
<Col xs={1} className="py-4">
<IconButton
icon="remove"
Copy link
Member

Choose a reason for hiding this comment

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

potentially see if we could make it wider / if bootstrap has an easy class name to do so, then let's do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added prop block={true}

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just block? True is inferred then.

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.

additionalProperties can't be added with semantic-ui fluent-ui themes
6 participants