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(conditional-format): support conditional format #1681

Merged
merged 157 commits into from
Mar 28, 2024

Conversation

Gggpound
Copy link
Contributor

No description provided.

@Gggpound Gggpound changed the title feat-conditional-format feat(cf): support conditional format Mar 25, 2024
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 20.26838% with 2852 lines in your changes are missing coverage. Please review.

Project coverage is 31.62%. Comparing base (eabe6fb) to head (871fd06).

Files Patch % Lines
...-format/src/components/panel/rule-edit/iconSet.tsx 4.00% 168 Missing ⚠️
...al-format/src/components/panel/rule-list/index.tsx 1.78% 165 Missing ⚠️
...rmat/src/services/calculate-unit/highlight-cell.ts 35.06% 150 Missing ⚠️
...rmat/src/components/panel/rule-edit/colorScale.tsx 2.20% 133 Missing ⚠️
...t/src/components/panel/rule-edit/highlightCell.tsx 2.98% 130 Missing ⚠️
...-format/src/services/conditional-format.service.ts 48.30% 122 Missing ⚠️
...format/src/controllers/cf.copy-paste.controller.ts 0.84% 118 Missing ⚠️
...al-format/src/components/panel/rule-edit/index.tsx 2.72% 107 Missing ⚠️
...-format/src/components/panel/rule-edit/dataBar.tsx 2.75% 106 Missing ⚠️
...src/services/conditional-format-formula.service.ts 31.37% 105 Missing ⚠️
... and 59 more
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1681      +/-   ##
==========================================
- Coverage   32.38%   31.62%   -0.76%     
==========================================
  Files         976     1039      +63     
  Lines       54179    57725    +3546     
  Branches    11286    12120     +834     
==========================================
+ Hits        17547    18258     +711     
- Misses      36632    39467    +2835     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Mar 25, 2024

View Deployment

📑 Examples 📚 Storybook
🔗 Preview link 🔗 Preview link

operator: IAverageHighlightCell['operator'];
}
// { ranges: [{ startRow: 0, endRow: 10, startColumn: 3, endColumn: 3 }, { startRow: 0, endRow: 10, startColumn: 5, endColumn: 5 }], style: { fs: 30 }, operator: 'greaterThan' };
export const addAverageCfCommand: ICommand<IAddAverageCfParams> = {
Copy link
Member

Choose a reason for hiding this comment

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

Command's name should start with a capical letter.

* limitations under the License.
*/

import type { IMutation } from '@univerjs/core';
Copy link
Member

Choose a reason for hiding this comment

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

File name should be in kebab case.

value: number;
}

export enum OPERATION {
Copy link
Member

Choose a reason for hiding this comment

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

The name should be more precise.

operator,
style,
} };
commandService.executeCommand(addConditionalRuleMutation.id, { unitId, subUnitId, rule } as IAddConditionalRuleMutationParams);
Copy link
Member

Choose a reason for hiding this comment

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

Should return the result of executing addConditionalRuleMutation.

});
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

return result

subUnitId: string;
rule: IConditionFormatRule;
}
export const addConditionalRuleMutationUndoFactory = (accessor: IAccessor, param: IAddConditionalRuleMutationParams) => {
Copy link
Member

Choose a reason for hiding this comment

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

Factories' names should start with a capital letter.

@@ -109,7 +109,7 @@ export const ToolbarItem = forwardRef((props: IDisplayMenuItem<IMenuItem>, ref:

const { selections } = props as IDisplayMenuItem<IMenuSelectorItem>;

const options = selections as IValueOption[];
Copy link
Member

Choose a reason for hiding this comment

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

Cannot use hooks with if logic.

import type { IConditionFormatRule, ITextHighlightCell } from '../models/type';

export const SHEET_CONDITION_FORMAT_PLUGIN = 'SHEET_CONDITION_FORMAT_PLUGIN';
export enum TextOperator {
Copy link
Member

@wzhudev wzhudev Mar 26, 2024

Choose a reason for hiding this comment

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

enum should have prefix in their names in avoid of conflicting with other features. For examples, auto filter has operators too.

ctx.globalCompositeOperation = 'destination-over';
Range.foreach(spreadsheetSkeleton.rowColumnSegment, (row, col) => {
const cellData = worksheet.getCell(row, col) as IDataBarCellData;
if (cellData && cellData.dataBar) {
Copy link
Member

Choose a reason for hiding this comment

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

Return earlier to make the code more readable.

* limitations under the License.
*/

/**
Copy link
Member

Choose a reason for hiding this comment

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

Two license headers.

@Gggpound Gggpound force-pushed the gggpound/feat-conditional-format-0325 branch from b3c5573 to 8dc574d Compare March 26, 2024 12:02
@Gggpound Gggpound added feature request New feature or request qa:untested This PR is ready to be tested labels Mar 26, 2024
@Gggpound Gggpound force-pushed the gggpound/feat-conditional-format-0325 branch from 8dc574d to a1064e1 Compare March 26, 2024 12:17
Copy link
Member

@jikkai jikkai left a comment

Choose a reason for hiding this comment

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

LGTM

@wzhudev wzhudev removed the feature request New feature or request label Mar 27, 2024
@Gggpound Gggpound force-pushed the gggpound/feat-conditional-format-0325 branch 2 times, most recently from 81abc8b to 87d80d1 Compare March 27, 2024 14:21
@jikkai jikkai changed the title feat(cf): support conditional format feat(conditional-format): support conditional format Mar 27, 2024
@Gggpound Gggpound force-pushed the gggpound/feat-conditional-format-0325 branch from 87d80d1 to 94dc6c1 Compare March 28, 2024 06:19
@Gggpound Gggpound force-pushed the gggpound/feat-conditional-format-0325 branch from 94dc6c1 to 871fd06 Compare March 28, 2024 07:14
@zhaolixin7 zhaolixin7 added the qa:verified This PR has already by verified by a QA and is considered good enough to be merge label Mar 28, 2024
@univer-bot univer-bot bot removed the qa:untested This PR is ready to be tested label Mar 28, 2024
@Gggpound Gggpound merged commit 50edd34 into dev Mar 28, 2024
8 checks passed
@Gggpound Gggpound deleted the gggpound/feat-conditional-format-0325 branch March 28, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa:verified This PR has already by verified by a QA and is considered good enough to be merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants