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

Add conditional button render props to EditableCard and EditableArrayCard components #574

Open
wants to merge 10 commits into
base: antd3
Choose a base branch
from

Conversation

ellenshin
Copy link
Contributor

@ellenshin ellenshin commented Aug 24, 2023

Description

Add disable props for EditableCard and EditableArrayCard in order to disable actions.
This change was needed to disable buttons within CoOp based on the user permission (see linked Jira ticket for more detail). Also added optional tooltips for disabled action buttons in order to enhance user experience.

JIRA: https://thatsmighty.atlassian.net/browse/PROD-1967

Original PR for disabling delete - which unfortunately did not work in CoOp due to antd versioning discrepancy (see note below) https://github.com/mighty-justice/coop-client/pull/1198. This change should be reverted for the time being.

Note: We are branching off of an older version of FieldsAnt in order to bypass the fact that coop-client is still using the old antd version. In the future, we should upgrade outdated libraries in CoOp.

Screenshots

Screen Shot 2023-08-28 at 4 28 33 PM Screen Shot 2023-08-28 at 4 29 34 PM

@ekatsuta ekatsuta changed the base branch from master to antd3 August 28, 2023 20:17
@ellenshin ellenshin changed the title Conditional btn render Add conditional button render props to EditableCard and EditableArrayCard components Aug 28, 2023
@cry-stal-lee cry-stal-lee self-requested a review August 29, 2023 18:50
);
}

private buttons () {
return (
<ButtonToolbar noSpacing>
{this.deleteButton}
<div style={{ display: 'inline-block', width: '10px' }}></div>
Copy link
Contributor

@cry-stal-lee cry-stal-lee Aug 29, 2023

Choose a reason for hiding this comment

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

Can we try using antd Space for this? i.e.

import { Space } from 'antd';

function MyComponent() {
  return (
    <Space size={10}>
      {this.deleteButton}
      {this.editButton}
    </Space>
  );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Space component isnt supported in antd3

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm could we try making a parent div with display: flex, gap: 10px, and justify-content: space-between and see if that works? Would like to try to avoid the empty <div> if possible, though if that doesn't work I'm OK with this.

it also looks like we don't have much custom styling in this library since we mostly just use the built-in antd functions, so I think inline styling is OK as a workaround for now since this is just a branch. ideally on the main branch we will eventually upgrade to antd4 and use <Space> for things like this.

Copy link
Contributor Author

@ellenshin ellenshin Aug 29, 2023

Choose a reason for hiding this comment

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

actually deleting the spans made this not needed. im seeing the tooltips when the buttons are disabled so not sure what you saw in the previous PR? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe antd3 vs 4 difference?

@cry-stal-lee cry-stal-lee self-requested a review August 29, 2023 20:17
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.

3 participants