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(grid): added export progress visualization in toolbar #8000

Merged
merged 15 commits into from
Aug 20, 2020

Conversation

igdmdimitrov
Copy link
Contributor

@igdmdimitrov igdmdimitrov commented Aug 14, 2020

Closes #7738

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@igdmdimitrov igdmdimitrov marked this pull request as ready for review August 17, 2020 14:02
@igdmdimitrov
Copy link
Contributor Author

@simeonoff, the export progress visualization is implemented in the grid toolbar but there is some work needed on styling. Let me know if you (or whoever is working on this) have any questions.

@@ -23,6 +23,10 @@
@extend %igx-grid-toolbar__actions !optional;
}

@include e(export-progress){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we name this something more generic. I think it might be used for other purposes as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about e(progress-bar)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think that's fine 👍

@desig9stein
Copy link
Contributor

desig9stein commented Aug 20, 2020

I've noticed an issue with different themes.
The progress bar has different sizes in different themes, and in bootstrap, for example, looks very ugly. I think that we should have some fixed height on the container that holds the progress bar in the toolbar. For me, 2px is more than enough. @StefanIvanov what do you think?

@StefanIvanov
Copy link
Contributor

@desig9stein and I had a call to look at the style and decided to go along with 2px high progress that is 1px below the bottom of the toolbar in order to overlap the border and look even better. I have updated the spec.

@@ -19,6 +19,7 @@ body {

@include igx-core($direction: ltr);
@include igx-theme($palette: $palette, $schema: $schema);
@include igx-theme($palette: $palette, $schema: $schema, $legacy-support: true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't make changes to this file.

@simeonoff simeonoff self-requested a review August 20, 2020 11:48
Copy link
Collaborator

@simeonoff simeonoff left a comment

Choose a reason for hiding this comment

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

Take a look at my comment.

Copy link
Collaborator

@simeonoff simeonoff left a comment

Choose a reason for hiding this comment

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

Haha, you still make changes to the file. Please, just revert all changes you made to the igniteui-theme.scss file.

simeonoff
simeonoff previously approved these changes Aug 20, 2020
@DiyanDimitrov DiyanDimitrov self-assigned this Aug 20, 2020
@DiyanDimitrov DiyanDimitrov added the ✅ status: verified Applies to PRs that have passed manual verification label Aug 20, 2020
@DiyanDimitrov DiyanDimitrov merged commit 02c3dc5 into master Aug 20, 2020
@DiyanDimitrov DiyanDimitrov deleted the dmdimitrov/export-progress branch August 20, 2020 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grid export visualization
5 participants