Skip to content

Conversation

satyasinha
Copy link

This requires more css changes

@imlucas imlucas changed the title Compass 221 match design create index modal COMPASS-221: DDL: Update Create Index Modal to Match new Designs Nov 1, 2016
@Sean-Oh
Copy link
Contributor

Sean-Oh commented Nov 2, 2016

Some comments on the interaction:

comment1


comment2

@satyasinha satyasinha force-pushed the COMPASS-221-Match-Design-Create-Index-Modal branch from 1129a13 to 53076de Compare November 2, 2016 23:54
@satyasinha
Copy link
Author

Just confirming, are you saying that when there are 2 or more rows the bottom one (with 'Select a field name') can be deleted by pressing the active - button?

@Sean-Oh
Copy link
Contributor

Sean-Oh commented Nov 3, 2016

Yeah, exactly!

Also, it seems like the process of adding a new field hasn't been updated. Users shouldn't have to press "add another" to save the field, as fields should be saved automatically. Let me know if this isn't clear, and we can talk it through.

Thanks!

@satyasinha
Copy link
Author

Ok that makes sense, I was wondering about that. So the add implementation is different now.

So pressing the add button would make a new row at the bottom with the zero state for field and type ('Select a field name' + 'Select a type'). Now does that mean there could be multiple zero state rows if the user keeps pressing the ADD FIELD button and not filling them out? Or should this be limited to at most 1.

@Sean-Oh
Copy link
Contributor

Sean-Oh commented Nov 3, 2016

Yeah - a user can press the add field without filling them out, so there could be multiple zero state rows at once. We could impose a reasonable limit to how many zero states can be shown at once. Also, if a user presses "create" without filling out their inputs, they'll see an error message along with those respective inputs highlighted in red.

@satyasinha satyasinha force-pushed the COMPASS-221-Match-Design-Create-Index-Modal branch from fba0087 to 8d18c39 Compare November 6, 2016 11:48
Copy link
Contributor

@rueckstiess rueckstiess left a comment

Choose a reason for hiding this comment

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

This looks good. Couple of small suggestions inline.

I fixed the following things:

  • adding a red border on field validation (I guess?)
  • the "add another" button is now oddly shifted to the side.

'loadIndexes',
'sortIndexes',
'triggerIndexCreation',
'updateField',
Copy link
Contributor

Choose a reason for hiding this comment

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

is the updateField action still used? If not, can you remove all occurrences of it?

handleFieldSelect(field) {
this.setState({field: field});
selectName(name) {
// this.setState({name: name});
Copy link
Contributor

Choose a reason for hiding this comment

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

remove obsolete code if not needed (this line and others below...)

fields: React.PropTypes.array.isRequired,
field: React.PropTypes.object.isRequired,
idx: React.PropTypes.number.isRequired,
remove: React.PropTypes.bool.isRequired
Copy link
Contributor

Choose a reason for hiding this comment

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

what does the remove boolean prop represent? if the remove button is enabled/disabled? If so, maybe a better name would be canBeRemoved or isRemovable or enableRemove or something like that.

const fieldUI = {
name: field.name === '' ? DEFAULT_FIELD.name : field.name,
type: field.type === '' ? DEFAULT_FIELD.type : field.type
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is much nicer than "misusing" the index name to represent the placeholder text. 👍

@rueckstiess
Copy link
Contributor

LGTM.

@rueckstiess rueckstiess merged commit 711d512 into master Nov 7, 2016
@rueckstiess rueckstiess deleted the COMPASS-221-Match-Design-Create-Index-Modal branch November 7, 2016 11:09
pzrq pushed a commit that referenced this pull request Nov 22, 2016
* COMPASS-221 changed heading for field options

* COMPASS-221 rough implementation of designs

* Quick style fixes

* COMPASS-221 mostly done, may need some css clean up

* COMPASS-221 added some extra actions

* COMPASS-221 implemented add create index from store to ui

* COMPASS-221 implemented updating field name and type

* COMPASS-221 set initial state for fields

* COMPASS-221 implemented remove index field

* COMPASS-221 added some error cases

* COMPASS-221 clean up a few files after refactoring

* COMPASS-221 added some test cases

* validation of index fields, button styling

* fix index store test.

* COMPASS-221 disable fields that are already used

* Added bottom margins for each row, truncate long field names in the dropdown, make highlight red same hex as error message

* COMPASS-221 made some minor fixes after review
@pzrq
Copy link
Contributor

pzrq commented Nov 22, 2016

Backported to 1.5-releases so COMPASS-98 could be backported cleanly.

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.

5 participants