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

New platform cleanup filters #34555

Merged
merged 16 commits into from
Apr 15, 2019
Merged

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Apr 4, 2019

Summary

Removing filters from ui/filters

  • label
  • rison
  • risonDecode
  • fieldType
  • uriescape
  • shortdots
  • moment

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [ ] Unit or functional tests were updated or added to match the most common scenarios
- [ ] This was checked for keyboard-only and screenreader accessibility

@stacey-gammon stacey-gammon mentioned this pull request Apr 4, 2019
94 tasks
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom lizozom marked this pull request as ready for review April 7, 2019 13:03
@lizozom lizozom requested review from mattkime, spalger and a team and removed request for spalger April 7, 2019 13:03
@lizozom
Copy link
Contributor Author

lizozom commented Apr 7, 2019

@elastic/kibana-platform please pay specific attention to the removal of the uriencode directive in src/legacy/ui/public/url/url.js

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor Author

lizozom commented Apr 10, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor Author

lizozom commented Apr 10, 2019

retest

@lizozom lizozom force-pushed the new-platform-cleanup-filters branch from 6de43c0 to 9501d3d Compare April 10, 2019 09:51
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

LGTM but we should find someone with experience regarding how this might fail in unexpected ways.

@lizozom lizozom requested a review from a team as a code owner April 11, 2019 14:32
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

}

return $parse(expr)(paramObj);
return encodeURIComponent($parse(expr)(paramObj));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we always need to encode this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A. It's always best to encode URL params
B. It's the previous behavior :)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested ML changes (data visualizer date card), and LGTM

@lizozom lizozom merged commit 71d9c5a into elastic:master Apr 15, 2019
lizozom added a commit to lizozom/kibana that referenced this pull request Apr 15, 2019
* Delete label filter
* Delete risonDecode
* Deleted rison
* removed fieldType filter
* Deleted field type tests
* Replaced uriescape filter with use of encodeURIComponent
* Delete short dots filter
* Removed usages of moment filter (and moved it to watcher)
* Code review changes
lizozom added a commit that referenced this pull request Apr 15, 2019
* Delete label filter
* Delete risonDecode
* Deleted rison
* removed fieldType filter
* Deleted field type tests
* Replaced uriescape filter with use of encodeURIComponent
* Delete short dots filter
* Removed usages of moment filter (and moved it to watcher)
* Code review changes
@lizozom lizozom deleted the new-platform-cleanup-filters branch April 21, 2019 10:45
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.

6 participants