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

Feature/paper menu #140

Merged
merged 12 commits into from
Sep 20, 2015
Merged

Conversation

peec
Copy link
Contributor

@peec peec commented Jul 26, 2015

Implementation:

  • All css is direct angular material (no modifications and such).
  • Some code for animations and such are from angular-material

Features:

  • Menues
    • Animations
    • Event handling ( arrow keys, focusing etc. )
    • All functionality of menu done
  • Select
    • Animations
    • Basic usage
    • Optgroups
    • Async
    • Event handling ( arrow keys, focusing etc. )

Menues

Current public api for component (in its simplest way):

      {{#paper-menu as |menu|}}
        {{#paper-button target=menu action="toggleMenu" icon-button=true}}{{paper-icon class="md-menu-origin" icon="local-phone"}}{{/paper-button}}
      {{else}}
        {{#paper-menu-item action="openSomething"}}
          {{paper-icon icon="adb" class="md-menu-align-target"}} Hello World!
        {{/paper-menu-item}}
      {{/paper-menu}}

Select

        {{#paper-select placeholder="State" model=userState}}
          {{#each states as |state|}}
            {{#paper-option value=state.abbrev}}
              {{state.abbrev}}
            {{/paper-option}}
          {{/each}}
        {{/paper-select}}

@jamesdixon
Copy link

@peec any idea when this might be merged? could really use the awesome select menu you created. great work!

@peec
Copy link
Contributor Author

peec commented Aug 21, 2015

@jamesdixon I'm not sure when, but #138 needs to be merged first (which is good to be merged in my eyes). When #138 is merged I will update this PR to conform with the new changes and relying on ember-css-transitions like #138 does.

@miguelcobain Maybe Miguel can answer this. Do you have any plans of when to review / merge #138 ?

bumpo

progressing.

progressing ...

Starting to get somewhere.

resuable components for menu / select etc. md-select (not finished).

Implement the select custom animation.

Remove the need of a wrapper inside menu component.

Support optgroups

wire events for selects, add async select with progress.

lint  + optional to turn off cache on async select.

Small additions to select, support disable

Use correct class if placeholder is active. Move click open to container itself

Start refactoring, move menu based features to menu, the rest is considered to be abstract for all 'menu' based components. This cleans up some wierd code within the first component (paper-select) and (paper-menu).

Rewamp menues docs and add border-box to prism override so the box does not break layout.

Resolve label issue in select when change of route instead pass it directly and not from the promise. Document paper-select.

The power of to='inverse' makes menu component much simpler.

Use spans around text in paper-menu-item.

Code style.

Add deps
@jamesdixon
Copy link

@peec thanks! Would it be OK to work off of your fork in the meantime? I just need to get something roughed out, so doesn't need to be perfect.

@peec
Copy link
Contributor Author

peec commented Aug 22, 2015

@jamesdixon Yes, that is no problem :) I have started some cleanups now on this PR so that it will be ready when #138 is on master.

@jamesdixon
Copy link

@peec awesome! which branch should i pull from in order to obtain the select functionality?

@jamesdixon
Copy link

@peec nevermind. glossed over it -- feature/paper-menu 👍

@jamesdixon
Copy link

@peec implemented your branch and things are looking great! Had a quick question to see if you had planned on supporting the placeholder transforming to a label after an option has been selected on the select menu? Also curious if it's trivial for the focused state to change the color of the line on the select menu to match the highlight of the other forms? Thank you!

@peec
Copy link
Contributor Author

peec commented Aug 26, 2015

@jamesdixon I will take a look at that, thanks for testing so we can fix this before final merge 👍

@jamesdixon
Copy link

@peec no problem! appreciate you developing this component!

Also, I did have an alignment issue out of the box:

image

Here's the associated HTML code, which is part of a standard 3-column bootstrap setup:

<div class="col-sm-3">
  <md-select id="ember738" tabindex="0" class="ember-view md-default-theme">
    <md-select-value id="ember747" class="ember-view md-select-value md-select-placeholder">
      <span>State</span>
      <span class="md-select-icon" aria-hidden="true"></span>
    </md-select-value>
  </md-select>
</div>

As far as I can see, the only styling applied is this from the Ember Paper stylesheet:

md-select {
    display: flex;
    margin: 20px 0 26px 0;
    margin-top: 20px;
    margin-right: 0px;
    margin-bottom: 26px;
    margin-left: 0px;
}

Cheers,
James

@peec
Copy link
Contributor Author

peec commented Aug 28, 2015

@jamesdixon Regarding alignment: I think the reason is you would have to use standard angular grid system here and wrap the inputs in <md-input-container>, I wouldn't recommend using bootstrap with material stylesheets as bootstrap does not use flexbox for grid, if you want grid like you should use <md-input-container flex="a value from 1 - 100">.

See https://github.com/peec/ember-paper/blob/feature/paper-menu/tests/dummy/app/templates/paper-select.hbs where all inputs are in a <md-input-container>.

Edit:
Here is the docs for the grid system we also use in ember-paper:
https://material.angularjs.org/latest/#/layout/grid

@jamesdixon
Copy link

@peec much appreciated! I didn't realize that ember-paper also implemented its own grid system. Thank you!

@miguelcobain
Copy link
Collaborator

So, is there something missing in this PR, @peec?

@jamesdixon
Copy link

@peec hate to bother you again, but I'm attempting to actually load data into the select menu and am running into an issue. I've followed the public API as you mentioned above, but unfortunately, it appears the select is rendering only a single item, which is just a string of objects. It ends up looking like this:

image

and generates the following output:

<md-select id="ember634" tabindex="0" class="ember-view md-default-theme">
  <md-select-value id="ember643" class="ember-view md-select-value">
    <span>[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object
      Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object
      Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object
      Object],[object Object],[object Object],[object Object]</span>
    <span class="md-select-icon" aria-hidden="true"></span>
  </md-select-value>
</md-select>

The code I'm using is the following:

{{#paper-select placeholder=(t 'forms.address.state') model=usStates.states}}
  {{#each state as |s|}}
    {{#paper-option value=s.abbrev}}
      {{s.name}}
    {{/paper-option}}
  {{/each}}
{{/paper-select}}

Where usStates.states is a service that returns an array of objects in the format of:

[{ name: 'Colorado', abbrev: 'CO' }, ...]

Just to test to make sure nothing was incorrect with my data, I switched it to an each helper without the surrounding paper-select code and all data was output properly.

If there is something fundamentally wrong with my approach or the format of the data, please let me know.

Also, on separate note, it appears the website where your demo lives periodically goes down throughout the day. Wasn't sure if it was on a VM or something that shuts down during specific times to save money.

Thank you and please let me know if there's anything I can do to assist.

Best,
James

@peec
Copy link
Contributor Author

peec commented Aug 29, 2015

@jamesdixon I see that states is passed to the model, the model attribute is the selected model (and not list of model).

I think you want to have something like this:

{{#paper-select placeholder=(t 'forms.address.state') model=selectedState}}
  {{#each usStates.states as |s|}}
    {{#paper-option value=s.abbrev}}
      {{s.name}}
    {{/paper-option}}
  {{/each}}
{{/paper-select}}

@peec
Copy link
Contributor Author

peec commented Aug 29, 2015

@miguelcobain I'll go through the PR now, and check if anything is missing, but this should be very near complete after much refactoring earlier.

…ch makes it compatible with paper-input. Uses paper-input has base.
@peec
Copy link
Contributor Author

peec commented Aug 29, 2015

@miguelcobain I refactored code style, and fixed wrap in md-input-container so that labels works correctly also for select (pops above) when has value. This should be ready now.

@jamesdixon
Copy link

@peec thank you! I knew it was something dumb I was doing. The param being called model was throwing me off. I was thinking that it was the model containing all of the states. Wondering if maybe that param should be renamed to selected or something a bit more telling more for newbies like me. Regardless, thank you for your help!

@jamesdixon
Copy link

@peec - it appears I may have spoken too soon. Using the code you provided above, I receive the following error when clicking on the select menu:

Uncaught Error: ember-wormhole failed to render into '#paper-wormhole' because the element is not in the DOM

When digging into the HTML, the list of options is still not rendered. I did pull down your latest commit, but unfortunately, it did not have any effect.

Sorry to be a pest, but not sure where to go from here. I did ensure that the list of states is accessible by doing a standard {{#each}} and they did display properly.

Thank you!

@peec
Copy link
Contributor Author

peec commented Aug 31, 2015

Regarding wormhole, some commits ago I changed to using wormhole - check
out the in the master branch of ember-paper, application.hbs: you need to
have the

empty tag inside
29. aug. 2015 19.15 skrev "jamesdixon" notifications@github.com:

@peec https://github.com/peec - it appears I may have spoken too soon.
Using the code you provided above, I receive the following error when
clicking on the select menu:

Uncaught Error: ember-wormhole failed to render into '#paper-wormhole'
because the element is not in the DOM

When digging into the HTML, the list of options is still not rendered. I
did pull down your latest commit, but unfortunately, it did not have any
effect.

Sorry to be a pest, but not sure where to go from here. I did ensure that
the list of states is accessible by doing a standard {{#each}} and they
did display properly.

Thank you!


Reply to this email directly or view it on GitHub
#140 (comment)
.

@jamesdixon
Copy link

Thanks, @peec. That also remedied my issue with the states not displaying.

There is still an alignment issue. I noticed that you had made a commit to wrap the select in md-input-container. That does fix the alignment when a value has been selected in the menu. However, when there is no value (just a placeholder), the alignment is still off. I was able to remedy it using the following CSS:

  md-input-container:not(.md-input-has-value) .md-select-value {
    padding-top: 26px;
  }

@jamesdixon
Copy link

@peec I didn't see it the demo, but wanted to ask if there was support for validation on selects?

@jamesdixon
Copy link

@peec hi again. I've confirmed that there is no way to display an error message or use validation at this point. I attempted to implement it myself, but I'm having trouble finding some sort of hook that is called when the menu is dismissed. My thinking was that validation should only be performed once the menu is open and then dismissed. Any thought or suggestions on this? I know you're busy, so I'd love to contribute. Thanks!

@jamesdixon
Copy link

@peec one other thing I noticed is that the select will display the list of items properly, but after choosing an item, the select displays the value of the selected item rather than displaying the label of the item.

peec added a commit that referenced this pull request Sep 20, 2015
Implement paper menu and select component
@peec peec merged commit 2a9857d into adopted-ember-addons:master Sep 20, 2015
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