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.

Sign up for GitHub

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

Jump to bottom

Add comments and inline docs for visualization saving and editing process. #7968

Closed
wants to merge 1 commit into from
Closed

Add comments and inline docs for visualization saving and editing process. #7968

wants to merge 1 commit into from

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented Aug 10, 2016 •
edited
Loading

Pertains to #3947

The goal is to clarify where URL state is coming from, when working with visualizations. Some of the classes touched upon are: SavedVis, PersistedState, AppState, and base classes.

EDIT: I guess I did make some small changes early on in the process, but they are pretty non-invasive.

My next step will be superficial refactoring by changing names to make intent/role clearer, without doing any invasive reorganization.

cjcenizal
cjcenizal reviewed Aug 10, 2016
View reviewed changes
src/core_plugins/kibana/public/visualize/editor/editor.js
const editableVis = vis.createEditableVis();

// NOTE: Why is this instance method being overwritten?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spalger Question

Copy link
Contributor

Choose a reason for hiding this comment

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

The editableVis is intended to be a mirror of the vis object, and as such it needs to go through the same lifecycle methods that vis does (in this case #requesting()). I'm not certain what benefits from the requesting method being called on the editable vis though...

@cjcenizal cjcenizal force-pushed the feature/internal-state-management branch from 3fc6daa to 93ca85a Compare August 10, 2016 00:17
cjcenizal
cjcenizal reviewed Aug 10, 2016
View reviewed changes
src/ui/public/vis/vis.js
*
* @description This class consists of aggs, params, listeners, title, and type.
* - Aggs: Instances of AggConfig.
* - Params: Are these the settings in the Options tab?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spalger Question

Copy link
Contributor

Choose a reason for hiding this comment

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

yep

@cjcenizal cjcenizal force-pushed the feature/internal-state-management branch 2 times, most recently from fabfa81 to 4c5ef75 Compare August 10, 2016 00:37
@cjcenizal
Add comments and inline docs for visualization saving and editing pro…
a5a4f04 
…cess.

The goal is to clarify where URL state is coming from, when working with
visualizations. Some of the classes touched upon are: SavedVis,
PersistedState, AppState, and base classes.

Explored files:
- src/core_plugins/kibana/public/visualize/editor/editor.js
- src/core_plugins/kibana/public/visualize/saved_visualizations/_saved_vis.js
- src/ui/public/courier/data_source/_doc_send_to_es.js
- src/ui/public/courier/data_source/doc_source.js
- src/ui/public/courier/saved_object/saved_object.js
- src/ui/public/es.js
- src/ui/public/events.js
- src/ui/public/persisted_state/persisted_state.js
- src/ui/public/state_management/app_state.js
- src/ui/public/state_management/state.js
- src/ui/public/vis/agg_config.js
- src/ui/public/vis/agg_configs.js
- src/ui/public/vis/vis.js
@cjcenizal cjcenizal force-pushed the feature/internal-state-management branch from 4c5ef75 to a5a4f04 Compare August 10, 2016 00:39
@spalger spalger removed their assignment Aug 23, 2016
@cjcenizal cjcenizal mentioned this pull request Sep 9, 2016
Merged
@cjcenizal cjcenizal closed this Sep 9, 2016
@cjcenizal cjcenizal deleted the feature/internal-state-management branch September 9, 2016 20:32
Ikuni17 pushed a commit that referenced this pull request Aug 31, 2024
@cee-chen @mbondyra
Upgrade EUI to v95.9.0 (#190752)
a88cfc8 
`v95.7.0` ⏩ `v95.9.0`

> [!CAUTION]
> This PR contains the final set of Emotion conversions for all EuiForm
components.
> If your plugin contains any custom CSS/styling to **EuiFormRow,
EuiFormControlLayout, EuiCheckbox, EuiRadio, or EuiSwitch**, ⚠️ make
sure to QA your UI to ensure no visual regressions have occurred! ⚠️

---

## [`v95.9.0`](https://github.com/elastic/eui/releases/v95.9.0)

- Updated `EuiSearchBar`'s optional `box.schema` prop with a new
`recognizedFields` configuration. This allows specifying the phrases
that will be parsed as field clauses
([#7960](elastic/eui#7960))
- Updated `EuiIcon` with a new `tokenSemanticText` glyph
([#7971](elastic/eui#7971))
- Added support for TypeScript 5
([#7980](elastic/eui#7980))

**Bug fixes**

- Fixed `EuiSelectableTemplateSitewide` styles when used within a
dark-themed `EuiHeader`
([#7977](elastic/eui#7977))

## [`v95.8.0`](https://github.com/elastic/eui/releases/v95.8.0)

- Updated `EuiHeaderLinks`'s mobile menu to set a slight popover padding
by default ([#7961](elastic/eui#7961))
- This can be overridden via `popoverProps.panelPaddingSize` if needed.
- Updated `EuiHeaderLink` to default to a size of `s` (down from `m`)
([#7961](elastic/eui#7961))

**Accessibility**

- Updated the `aria-label` attribute for the `EuiFieldSearch` clear
button ([#7970](elastic/eui#7970))

**Bug fixes**

- Fixed a visual bug with `<EuiDualRange showInput="inputWithPopover"
/>` form controls ([#7957](elastic/eui#7957))

**Deprecations**

- Deprecated `EuiFormRow`'s `columnCompressedSwitch` display prop. Use
`columnCompressed` instead, which will automatically account for child
`EuiSwitch`es ([#7968](elastic/eui#7968))
- Deprecated `EuiFormRow`'s `rowCompressed` display prop. Use `row`
instead for vertical forms, or `centerCompressed` for inline forms
([#7968](elastic/eui#7968))
- (Styling) Updated `EuiFormRow`'s `hasEmptySpaceLabel` prop to no
longer attempt to automatically align its content to a vertical center.
Use the `display="center"` prop for that instead
([#7968](elastic/eui#7968))

**CSS-in-JS conversions**

- Converted `EuiFormControlLayout` to Emotion
([#7954](elastic/eui#7954))
- Removed `.euiFormControlLayout--*icons` classNames and
`--eui-form-control-layout-icons-padding` CSS var. Use
`--euiFormControlRightIconsCount` or `--euiFormControlLeftIconsCount`
instead
- Converted `EuiFormLayoutDelimited` to Emotion
([#7957](elastic/eui#7957))
- Fixed `cloneElementWithCss` throwing an error when used multiple times
without a `key` prop ([#7957](elastic/eui#7957))
- Updated `cloneElementWithCss` utility to support a third argument that
allows prepending vs. appending the cloned Emotion css className
([#7957](elastic/eui#7957))
- Removed `@euiFormControlLayoutClearIcon` Sass mixin
([#7959](elastic/eui#7959))
- Converted `EuiDescribedFormGroup` to Emotion
([#7964](elastic/eui#7964))
- Converted `EuiForm`, `EuiFormHelpText`, and `EuiFormErrorText` to
Emotion ([#7966](elastic/eui#7966))
- Converted `EuiFormLabel` and `EuiFormLegend` to Emotion; Removed
`@euiFormLabel` mixin
([#7967](elastic/eui#7967))
- Converted `EuiFormRow` to Emotion
([#7968](elastic/eui#7968))
- Converted `EuiCheckbox` to Emotion
([#7969](elastic/eui#7969))
- Converted `EuiRadio` to Emotion
([#7969](elastic/eui#7969))
- Converted `EuiSwitch` to Emotion
([#7969](elastic/eui#7969))
- Removed the following Sass variables:
([#7969](elastic/eui#7969))
  - `$euiFormCustomControlDisabledIconColor`
  - `$euiFormCustomControlBorderColor`
  - `$euiRadioSize`
  - `$euiCheckBoxSize`
  - `$euiCheckboxBorderRadius`
  - `$euiSwitchHeight` (and compressed/mini variants)
  - `$euiSwitchWidth` (and compressed/mini variants)
  - `$euiSwitchThumbSize` (and compressed/mini variants)
  - `$euiSwitchIconHeight`
  - `$euiSwitchOffColor`
- Removed the following Sass mixins:
([#7969](elastic/eui#7969))
  - `euiIconBackground`
  - `euiCustomControl`
  - `euiCustomControlSelected`
  - `euiCustomControlDisabled`
  - `euiCustomControlFocused`

---------

Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cjcenizal @spalger @tylersmalley