fix: make tokenizer/multiinput responsive #2086

mikerodonnell89 · 2020-02-28T19:23:03Z

Please provide a link to the associated issue.

fixes #2018

Please provide a brief summary of this pull request.

Collapse/expand tokens within the tokenizer depending on tokenizer width. Multi-input should display "____ more" text indicating how many collapsed tokens there are. Also refactor the breadcrumb responsiveness a little bit.

Please check whether the PR fulfills the following requirements

the commit message follows the guideline:
https://github.com/SAP/fundamental-ngx/blob/master/CONTRIBUTING.md
tests for the changes that have been done

netlify · 2020-02-28T19:27:34Z

Deploy preview for fundamental-ngx ready!

Built with commit 6788f2d

https://deploy-preview-2086--fundamental-ngx.netlify.com

estelaxu3 · 2020-03-04T21:38:52Z

I notice that if I delete the items in tokenizer, the label "x more" doesn't change. If I added 5 items, it will say "5 more" even when I delete everything.

stefanoScalzo · 2020-03-05T00:32:19Z

dont think we should allow empty tokens, it creates weird spacing

… using keyboard

mikerodonnell89 · 2020-03-13T21:54:01Z

Depends on SAP/fundamental-styles#775

mikerodonnell89 · 2020-04-07T00:14:24Z

Should have left this as blocked as it depends on #2228

estelaxu3 · 2020-04-07T14:50:01Z

The cozy mode can't be fold into "(x) more"

mikerodonnell89 · 2020-04-07T16:28:23Z

The cozy mode can't be fold into "(x) more"

this is according to the designs

mikerodonnell89 · 2020-04-07T16:48:51Z

This PR is changing many other files not just Tokenizer/MultiInput. Is it intentional?

A lot of the changes came from #2228 which has now been merged so you should be seeing fewer. There are still some seemingly random changes, like one to breadcrumb, but i did that because the tokenizer and breadcrumb resize logic is very similar.

InnaAtanasova · 2020-04-07T20:29:52Z

Mutli Input - there is a spacing on the right side of the button

The focus outline is cut

For MultiInput I think the button addon should be always visible. Now when you add more tokens it hides.

The Token has cut focus outline:

Shouldn't clicking on __more button open a dropdown with all Tokens?

JKMarkowski

Hello Mike, let's take a look at my comments. The example works well, but there are some changes to include in the code

JKMarkowski · 2020-04-08T09:45:28Z

libs/core/src/lib/form/form-control/form-control.directive.ts

+    buildComponentCssClass(): string {
+        return [
+            this.state ? 'is-' + this.state : '',
+            this.type === 'radio' ? 'fd-radio' : '',
+            this._getElementTag() === 'input' ? 'fd-input' : '',
+            this._getElementTag() === 'textarea' ? 'fd-textarea' : '',
+            this._getElementTag() === 'select' ? 'fd-form-select' : '',
+            this.compact && this._getElementTag() === 'input' ? 'fd-input--compact' : '',
+            this.compact && this._getElementTag() === 'textarea' ? 'fd-textarea--compact' : '',
+            this.compact && this._getElementTag() === 'select' ? 'fd-form-select--compact' : '',
+            this.class
+        ].filter(x => x !== '').join(' ');
+    }


Can we have some method there?

Suggested change

buildComponentCssClass(): string {

return [

this.state ? 'is-' + this.state : '',

this.type === 'radio' ? 'fd-radio' : '',

this._getElementTag() === 'input' ? 'fd-input' : '',

this._getElementTag() === 'textarea' ? 'fd-textarea' : '',

this._getElementTag() === 'select' ? 'fd-form-select' : '',

this.compact && this._getElementTag() === 'input' ? 'fd-input--compact' : '',

this.compact && this._getElementTag() === 'textarea' ? 'fd-textarea--compact' : '',

this.compact && this._getElementTag() === 'select' ? 'fd-form-select--compact' : '',

this.class

].filter(x => x !== '').join(' ');

}

buildComponentCssClass(): string {

return [

this.state ? 'is-' + this.state : '',

this._getFormClass(),

this.compact ? (this._getFormClass() + '--compact') : '',

this.class

].filter(x => x !== '').join(' ');

}

private _getFormClass(): string {

switch (this._getElementTag()) {

case: 'input':

return `fd-input`;

case: 'select':

...

}

}

JKMarkowski · 2020-04-08T09:47:29Z

libs/core/src/lib/token/tokenizer.component.ts

+    }
+
+    /** @hidden */
+    expandTokens(): void {


Can we make it private?

JKMarkowski · 2020-04-08T09:47:42Z

libs/core/src/lib/token/tokenizer.component.ts

+    }
+
+    /** @hidden */
+    collapseTokens(side?: string): void {


Can we make it private?

JKMarkowski · 2020-04-08T09:48:44Z

libs/core/src/lib/token/tokenizer.component.ts

    /** @hidden */
-    focusTokenElement(event: KeyboardEvent, newIndex: number): HTMLElement {
+    focusTokenElement(event: Event, newIndex: number): HTMLElement {


Order of methods rules should be followed, take a look there

JKMarkowski · 2020-04-08T09:50:31Z

libs/core/src/lib/token/tokenizer.component.html

        <ng-content select="fd-token"></ng-content>
+        <span *ngIf="(moreTokensLeft.length > 0 || moreTokensRight.length > 0) && compact" class="fd-tokenizer-more" #moreElement>
+            <ng-container>{{moreTokensLeft.length + moreTokensRight.length}} more</ng-container>


Can we put some customizable {{word}}, instead of more?

JKMarkowski · 2020-04-08T09:52:06Z

libs/core/src/lib/token/tokenizer.component.html

        <ng-content select="fd-token"></ng-content>
+        <span *ngIf="(moreTokensLeft.length > 0 || moreTokensRight.length > 0) && compact" class="fd-tokenizer-more" #moreElement>


I'm not sure if it's good idea to depend behaviour on compact flag. Maybe let's have another better docummented flag like collapseTokens: boolean or something. It would be great to put some dedicated section only for it

Are you saying collapseTokens would be an input or somehow behave separately from the compact flag? The specs make it pretty clear that phones are cozy and don't have "more" text and desktop is compact and does have "more" so I don't think we should deviate from that

This reverts commit ab2a011.

mikerodonnell89 added 5 commits February 27, 2020 17:12

start working on tokenizer responsiveness

6d3ee60

improved collapse method

8941107

tests

56ed97d

Merge branch 'master' into fix/responsive-tokenizer

a5fd9ab

fix more tests

5b09ab6

mikerodonnell89 requested a review from a team February 28, 2020 19:23

add more comments

57458ab

Merge branch 'master' into fix/responsive-tokenizer

4f62588

InnaAtanasova requested review from estelaxu3, salarenko, rengare, droshev and InnaAtanasova March 4, 2020 20:04

mikerodonnell89 changed the title ~~fix: responsive tokenizer~~ [WIP] fix: responsive tokenizer Mar 5, 2020

mikerodonnell89 changed the title ~~[WIP] fix: responsive tokenizer~~ fix: responsive tokenizer Mar 6, 2020

mikerodonnell89 changed the title ~~fix: responsive tokenizer~~ fix: make tokenizer responsive Mar 6, 2020

mikerodonnell89 changed the title ~~fix: make tokenizer responsive~~ [WIP] fix: make tokenizer responsive Mar 6, 2020

tokenizer responsiveness progress, "more" dropdown needs work

6f8bd41

mikerodonnell89 force-pushed the fix/responsive-tokenizer branch from 800f64d to 6f8bd41 Compare March 6, 2020 22:25

mikerodonnell89 changed the title ~~[WIP] fix: make tokenizer responsive~~ [WIP] fix: make tokenizer/multiinput responsive Mar 9, 2020

mikerodonnell89 added 5 commits March 9, 2020 13:53

only show "more" on multi-input

5dbce72

start keyboard support - weird behavior after removing one token then…

8236198

… using keyboard

couple tweaks

2ffe1e8

Merge branch 'master' into fix/responsive-tokenizer

a465010

better docs, alignment with changes to fd-styles

6380b32

mikerodonnell89 self-assigned this Mar 13, 2020

pr comments

e3c847c

mikerodonnell89 added the blocked blocked ticket label Apr 7, 2020

mikerodonnell89 changed the title ~~fix: make tokenizer/multiinput responsive~~ [blocked] fix: make tokenizer/multiinput responsive Apr 7, 2020

mikerodonnell89 added 2 commits April 7, 2020 08:26

fix conflicts

24092f4

accidental commas

5be1b66

mikerodonnell89 changed the title ~~[blocked] fix: make tokenizer/multiinput responsive~~ fix: make tokenizer/multiinput responsive Apr 7, 2020

mikerodonnell89 removed the blocked blocked ticket label Apr 7, 2020

mikerodonnell89 added 2 commits April 7, 2020 14:50

Merge branch 'master' into fix/responsive-tokenizer

efee5b2

fix multi-input addon issues

7b89e18

Development automation moved this from In progress to Review in progress Apr 8, 2020

JKMarkowski suggested changes Apr 8, 2020

View reviewed changes

mikerodonnell89 added 4 commits April 8, 2020 13:45

demonstrate new tokens in multi-input

110990c

make multi input bg transparent to fix outline issue

f892434

Merge branch 'master' into fix/responsive-tokenizer

a679295

netlify rebuild

c46a773

InnaAtanasova approved these changes Apr 8, 2020

View reviewed changes

JKMarkowski self-requested a review April 9, 2020 13:30

Development automation moved this from Review in progress to Reviewer approved Apr 9, 2020

JKMarkowski approved these changes Apr 9, 2020

View reviewed changes

mikerodonnell89 added 2 commits April 9, 2020 09:40

display more text on mobile

ab2a011

Revert "display more text on mobile"

6788f2d

This reverts commit ab2a011.

mikerodonnell89 merged commit 35d50ec into master Apr 9, 2020

Development automation moved this from Reviewer approved to Done Apr 9, 2020

droshev deleted the fix/responsive-tokenizer branch April 10, 2020 14:40

mikerodonnell89 mentioned this pull request Apr 14, 2020

Multi Input - Responsive behavior not according to Fiori guidelines #2291

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: make tokenizer/multiinput responsive #2086

fix: make tokenizer/multiinput responsive #2086

mikerodonnell89 commented Feb 28, 2020 •

edited by InnaAtanasova

netlify bot commented Feb 28, 2020 •

edited

estelaxu3 commented Mar 4, 2020

stefanoScalzo commented Mar 5, 2020

mikerodonnell89 commented Mar 13, 2020

mikerodonnell89 commented Apr 7, 2020

estelaxu3 commented Apr 7, 2020

mikerodonnell89 commented Apr 7, 2020

mikerodonnell89 commented Apr 7, 2020

InnaAtanasova commented Apr 7, 2020

JKMarkowski left a comment •

edited

JKMarkowski Apr 8, 2020 •

edited

JKMarkowski Apr 8, 2020

JKMarkowski Apr 8, 2020

JKMarkowski Apr 8, 2020

JKMarkowski Apr 8, 2020

JKMarkowski Apr 8, 2020

mikerodonnell89 Apr 8, 2020

		<ng-content select="fd-token"></ng-content>
		<span *ngIf="(moreTokensLeft.length > 0 \|\| moreTokensRight.length > 0) && compact" class="fd-tokenizer-more" #moreElement>

fix: make tokenizer/multiinput responsive #2086

fix: make tokenizer/multiinput responsive #2086

Conversation

mikerodonnell89 commented Feb 28, 2020 • edited by InnaAtanasova

Please provide a link to the associated issue.

Please provide a brief summary of this pull request.

Please check whether the PR fulfills the following requirements

netlify bot commented Feb 28, 2020 • edited

estelaxu3 commented Mar 4, 2020

stefanoScalzo commented Mar 5, 2020

mikerodonnell89 commented Mar 13, 2020

mikerodonnell89 commented Apr 7, 2020

estelaxu3 commented Apr 7, 2020

mikerodonnell89 commented Apr 7, 2020

mikerodonnell89 commented Apr 7, 2020

InnaAtanasova commented Apr 7, 2020

JKMarkowski left a comment • edited

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020 • edited

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020

Choose a reason for hiding this comment

JKMarkowski Apr 8, 2020

Choose a reason for hiding this comment

mikerodonnell89 Apr 8, 2020

Choose a reason for hiding this comment

mikerodonnell89 commented Feb 28, 2020 •

edited by InnaAtanasova

netlify bot commented Feb 28, 2020 •

edited

JKMarkowski left a comment •

edited

JKMarkowski Apr 8, 2020 •

edited