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

Bug: calcite-combobox: long strings arent formatted correctly #2096

Closed
2 tasks
kevindoshier opened this issue May 4, 2021 · 22 comments
Closed
2 tasks

Bug: calcite-combobox: long strings arent formatted correctly #2096

kevindoshier opened this issue May 4, 2021 · 22 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.

Comments

@kevindoshier
Copy link
Contributor

kevindoshier commented May 4, 2021

Actual Behavior

calcite-combobox: long strings arent formatted correctly

image

Expected Behavior

Shouldnt mess up the formatting of the combobox

Reproduction Steps or Sample

Relevant Info

Version: @esri/calcite-components@<version>

  • CDN
  • NPM package
@kevindoshier kevindoshier added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. labels May 4, 2021
@kevindoshier kevindoshier changed the title Bug: calcite-combobox: long strings dont arent formatted correctly Bug: calcite-combobox: long strings arent formatted correctly May 4, 2021
@driskull
Copy link
Member

driskull commented May 4, 2021

@julio8a who would this be for?

@julio8a
Copy link

julio8a commented May 6, 2021

Does this look like this might be part of refactoring? @caripizza

If it is, we should add it to that refactor epic.

@driskull
Copy link
Member

driskull commented Jun 9, 2021

I think we should elevate this, mapviewer can't use this for layer selector until this is fixed. @julio8a @jcfranco @caripizza

@jcfranco
Copy link
Member

jcfranco commented Jun 9, 2021

@driskull @kevindoshier It's not quite clear to me from the screenshot, but does this codepen represent the issue? https://codepen.io/jcfranco/pen/YzZOXNp?editors=1000

@driskull
Copy link
Member

driskull commented Jun 9, 2021

@jcfranco yes but also when the selection mode is single

@jcfranco
Copy link
Member

jcfranco commented Jun 9, 2021

Got it. Thanks for the info. Adding it to this sprint as help wanted. cc @caripizza

@jcfranco jcfranco added this to the Sprint 6/7 – 6/18 milestone Jun 9, 2021
@jcfranco jcfranco added the help wanted Issues that the core team needs help with in a sprint. label Jun 9, 2021
@driskull
Copy link
Member

I think a designer needs to take this issue since it's mostly a design decision on what to do and then updating the SCSS.

@caripizza
Copy link
Contributor

@kevindoshier @driskull Can you please add the code snippet that shows the issue? I'm having trouble replicating

@kevindoshier
Copy link
Contributor Author

Doesnt this one reproduce it?

#2096 (comment)

@caripizza
Copy link
Contributor

Thanks @kevindoshier, I missed that comment. Can you add repro steps to the Description to help make it more clear? Or is this just a question of what it should look like when it's refactored to wrap?

@kevindoshier
Copy link
Contributor Author

Its just about needing it to be styled correctly

@bstifle
Copy link

bstifle commented Jun 15, 2021

Couple of options for this:

image

@caripizza
Copy link
Contributor

@driskull - I chatted with @bstifle about this just now. What if we just truncate the selected item's text label with ellipses ("...") if it's over 25 characters long?

private trimLabels = (textLabel, chars = 25) => {
  return textLabel.length > chars ? textLabel.substring(0, chars) + "..." : textLabel;
};

Here's what that looks like on master without any extra styles:
image

The list item would still show the full label text like so:

image

image

image

Here's the markup I used for these screenshots:

    <div style="width:350px;margin-left:1rem;">
      <h3>Multi</h3>
      <calcite-combobox
        label="demo combobox 1"
        selection-mode="multi"
        allow-custom-values
      >
        <calcite-combobox-item value="Trees" text-label="Trees">
          <calcite-combobox-item
            value="CommercialDamageAssessment - Damage to Commercial Buildings & Damage to Commercial Buildings"
            text-label="CommercialDamageAssessment - Damage to Commercial Buildings & Damage to Commercial Buildings"
            selected
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Sequoia"
            disabled
            text-label="Sequoia"
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Douglas Fir"
            text-label="Douglas Fir"
          ></calcite-combobox-item>
        </calcite-combobox-item>
      </calcite-combobox>
    </div>

    <div style="width:350px;margin-left:1rem;">
      <h3>Ancestors</h3>
      <calcite-combobox
        label="demo combobox 2"
        selection-mode="ancestors"
        allow-custom-values
      >
        <calcite-combobox-item value="Trees" text-label="Trees">
          <calcite-combobox-item
            value="CommercialDamageAssessment - Damage to Commercial Buildings & Damage to Commercial Buildings"
            text-label="CommercialDamageAssessment - Damage to Commercial Buildings & Damage to Commercial Buildings"
            selected
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Sequoia"
            disabled
            text-label="Sequoia"
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Douglas Fir"
            text-label="Douglas Fir"
          ></calcite-combobox-item>
        </calcite-combobox-item>
      </calcite-combobox>
    </div>

    <div style="width:350px;margin-left:1rem;">
      <h3>Single</h3>
      <calcite-combobox
        label="demo combobox 3"
        selection-mode="single"
        allow-custom-values
      >
        <calcite-combobox-item value="Trees" text-label="Trees">
          <calcite-combobox-item
            value="CommercialDamageAssessment - Damage to Commercial Buildings"
            text-label="CommercialDamageAssessment - Damage to Commercial Buildings"
            selected
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Sequoia"
            disabled
            text-label="Sequoia"
          ></calcite-combobox-item>
          <calcite-combobox-item
            value="Douglas Fir"
            text-label="Douglas Fir"
          ></calcite-combobox-item>
        </calcite-combobox-item>
        <calcite-combobox-item
          value="Rivers"
          text-label="Rivers"
        ></calcite-combobox-item>
      </calcite-combobox>
    </div>

cc @macandcheese @bpatterson88

@caripizza caripizza added the needs triage Planning workflow - pending design/dev review. label Jun 15, 2021
@driskull
Copy link
Member

That sounds reasonable to me.

@driskull
Copy link
Member

Maybe make this a domUtil as well?

export const truncate = (text: string, chars = 25) => {
  return text.length > chars ? text.substring(0, chars) + "&hellip;" : text;
};

@driskull
Copy link
Member

@caripizza actually, can we just use CSS to do this? Seems like we can just use the truncate tailwind class and set a max-width on the tags and selected item?

@caripizza
Copy link
Contributor

@driskull I think with just CSS, we'd have to make sure the chip's close button stays visible too. I prefer the domUtil approach, since many components could opt into this trim pattern.

@driskull
Copy link
Member

I think we can do that with css still. The text already has a container inside the chip.

@caripizza
Copy link
Contributor

@driskull can I assign this to you then? It sounds like you got it working

@driskull
Copy link
Member

Sure, ill take a stab at it

@caripizza caripizza assigned caripizza and driskull and unassigned caripizza Jun 15, 2021
@caripizza caripizza removed the 0 - new New issues that need assignment. label Jun 15, 2021
@caripizza caripizza added the 1 - assigned Issues that are assigned to a sprint and a team member. label Jun 15, 2021
driskull added a commit that referenced this issue Jun 16, 2021
* fix(combobox): Truncate long strings correctly. #2096

* review fix

* revert title

* label de chip.
@driskull driskull assigned kevindoshier and unassigned driskull Jun 16, 2021
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Jun 16, 2021
@driskull
Copy link
Member

Installed.

@jcfranco jcfranco removed needs triage Planning workflow - pending design/dev review. help wanted Issues that the core team needs help with in a sprint. labels Jun 21, 2021
@julio8a
Copy link

julio8a commented Jun 22, 2021

Verified on master.

@julio8a julio8a closed this as completed Jun 22, 2021
@julio8a julio8a added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.
Projects
None yet
Development

No branches or pull requests

6 participants