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

perf(item-sliding): remove duplicate class #11829

Merged
merged 6 commits into from
Jun 2, 2017

Conversation

AmitMY
Copy link
Contributor

@AmitMY AmitMY commented May 28, 2017

Short description of what this resolves:

Removes another unneeded usage of the words left / right -> brings me calm :)

Changes proposed in this pull request:

  • Remove duplicate class for each side, as it is a clone

Ionic Version: 3.x

}

&.active-options-right ion-item-options:not([side=left]) {
&.active-options ion-item-options {
Copy link
Contributor

Choose a reason for hiding this comment

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

the active-options-left ion-item-options[side=left] were exists so that we only make it visible when it should be visible; when the client is about to slide the item to the right so the left option should be appear.
with this you will display things when user is about to swap whether left or right! doesn't matter! the options should be appear, in this case we will face a bug when the options are expandable and we have both left and right options and the user swap the item to the left user might see the other options on the left side as well (this can be easily reproducible just put three or four options to the left and one expandable to the right and pull your fingers from right to left) which will consider as a bug, I've spent a full day to figure these out :))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wow ok I didn't realise. So I'll just merge the selector instead of replacing it

@brandyscarney
Copy link
Member

This fails the Sass linter, should be on two lines and there are two { brackets:

[13:49:37] 1 issues found in /Users/brandyscarney/Documents/git/ionic/src/components/item/item-sliding.scss
[13:49:37] components/item/item-sliding.scss:83 [E] Syntax Error: Invalid CSS after "...[side=left]) { ": expected "}", was "{"
[13:49:37] 'lint.sass' errored after 13 s
[13:49:37] Error in plugin 'gulp-scss-lint'
Message:
    ScssLint failed for: components/item/item-sliding.scss

@AmitMY
Copy link
Contributor Author

AmitMY commented Jun 2, 2017

oops, fixed

@brandyscarney brandyscarney merged commit c9cb9ae into ionic-team:master Jun 2, 2017
@brandyscarney
Copy link
Member

👍

AmitMY added a commit to AmitMY/ionic that referenced this pull request Jun 6, 2017
* fix(lint): unused import

* perf(item-sliding): remove duplicate class

* fix(item-sliding): revert and merge selector

* fix(item-sliding): pass lint
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