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

More decorator improvements #2674

Merged
merged 10 commits into from Aug 15, 2016

Conversation

Projects
None yet
2 participants
@edmundoa
Member

edmundoa commented Aug 12, 2016

Here are some other improvements in message decorators:

  • Do not return internal fields in the field list. Fixes #2519
  • Maintain decorators sort after editing one of them, by submitting the order field to the server
  • Make decorator sidebar scrollable, allowing users to access all decorators if there are many or the screen height is not large enough
  • Attempt to resolve configuration IDs from the type definition. This allows us to show names in the configuration values instead of IDs

edmundoa added some commits Aug 10, 2016

Refactor SearchSidebar
We need to calculate the height for the sidebar when displaying
decorators or field analyzers, so it's time to clean this up, as it's
becoming too big and complicated.

This splits up the field analyzer section of the search sidebar, so it's
independent of the common controls.
Resolve entity IDs from decorator type definitions
Attempt to resolve IDs from the decorator type definitions if possible.
This allow us to display entity names (like we do in the configuration
forms) instead of simply display an ID.
@dennisoelkers

This comment has been minimized.

There is a typo in the file name: FieldAnalizersSidebar => FieldAnalyzersSidebar

This comment has been minimized.

Member

edmundoa replied Aug 12, 2016

I'll fix it, thank you!

edmundoa added some commits Aug 12, 2016

Make decorator list scrollable
When there are many search decorators in the sidebar it is not possible
to get to the non-visible ones.

This change adapts the sidebar height in a similar way as we do for
field analyzers, so at least it is possible to get to all the decorators
that are applied to the search results.
Remove internal fields when search is decorated
When a search is decorated, remove the internal fields from field set in
the search result response.

Fixes #2519
Keep decorator order after updated
When a decorator configuration is updated, submit its previous order so
the updated decorator still holds the same position.
Improve decorator sidebar scrolling
- Move styles into css file
- Give some padding to the scrollable component, so all options in the
  last decorator dropdown are still visible
- Use more height for the sidebar

@edmundoa edmundoa added this to the 2.1.0 milestone Aug 12, 2016

@dennisoelkers dennisoelkers self-assigned this Aug 12, 2016

configKeys.forEach(key => {
const configValues = (typeConfig[key] ? typeConfig[key].additional_info.values : undefined);
if (typeof configValues === 'object' && !Array.isArray(configValues)) {

This comment has been minimized.

@dennisoelkers

dennisoelkers Aug 12, 2016

Member

Why not just if (configValues && configValues[config[key]]) {?

This comment has been minimized.

@edmundoa

edmundoa Aug 12, 2016

Member

I was being too cautious in case some other configuration field was using values for something else. I'll change the code 👍

})
.stream()
.filter(field -> !Message.RESERVED_FIELDS.contains(field))
.collect(Collectors.toSet());

This comment has been minimized.

@dennisoelkers

dennisoelkers Aug 12, 2016

Member

This could be simplified to:

return messages.stream()
                .flatMap(message -> message.message().keySet().stream())
                .filter(field -> !Message.RESERVED_FIELDS.contains(field))
                .collect(Collectors.toSet());

which is easier to read imo.

This comment has been minimized.

@edmundoa

edmundoa Aug 12, 2016

Member

Looks much better indeed.

@dennisoelkers

This comment has been minimized.

Member

dennisoelkers commented Aug 15, 2016

LGTM! 👍

@dennisoelkers dennisoelkers merged commit fc0adc9 into master Aug 15, 2016

4 checks passed

ci-server-integration Jenkins build graylog2-server-integration-pr 1265 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 748 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dennisoelkers dennisoelkers deleted the decorator-improvements branch Aug 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment