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

fix(block): removes extra loading indicator #7239

Merged
merged 8 commits into from
Jun 29, 2023
Merged

fix(block): removes extra loading indicator #7239

merged 8 commits into from
Jun 29, 2023

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jun 28, 2023 •
edited
Loading

Related Issue: #6485

Summary

calcite-block header had two loading indicators. This removes the extra calcite-loader from the right end, and places it as an option in icon-slot,statusIcons: valid or invalid as well as controls now appear regardless of loading state.

@Elijbet Elijbet requested a review from a team as a code owner June 28, 2023 22:00
@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 28, 2023
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed bug Bug reports for broken functionality. Issues should include a reproduction of the bug. labels Jun 28, 2023
driskull
driskull approved these changes Jun 28, 2023
View reviewed changes
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 28, 2023
Elijbet added 2 commits June 28, 2023 15:48
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 28, 2023
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 29, 2023
jcfranco
jcfranco approved these changes Jun 29, 2023
View reviewed changes
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome!

packages/calcite-components/src/components/block/resources.ts
@@ -35,6 +34,5 @@ export const ICONS = {
opened: "chevron-up",
closed: "chevron-down",
valid: "check-circle",
invalid: "exclamation-mark-triangle",
refresh: "refresh"
Copy link
Member

Choose a reason for hiding this comment

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

✨🧹✨

@benelan benelan merged commit a334a75 into master Jun 29, 2023
@benelan benelan deleted the elijbet/6485-blcok-too-many-indicators branch June 29, 2023 20:44
@github-actions github-actions bot added this to the 2023 July Priorities milestone Jun 29, 2023
@calcite-admin calcite-admin mentioned this pull request Jun 29, 2023
Closed
@geospatialem
Copy link
Member

It looks like the block's icon slot isn't getting overridden in this case per #6485 (comment) in the updated sample.

@Elijbet Is this the expected structure and behavior code-wise? cc @ashetland

@jcfranco
Copy link
Member

jcfranco commented Jun 30, 2023 •
edited
Loading

@geospatialem I noticed the sample above uses 1.5.0-next.8, could you try next or 1.5.0-next.9 instead?

@calcite-admin calcite-admin mentioned this pull request Jul 1, 2023
Merged
@geospatialem
Copy link
Member

geospatialem commented Jul 3, 2023 •
edited
Loading

@geospatialem I noticed the sample above uses 1.5.0-next.8, could you try next or 1.5.0-next.9 instead?

Ah, great catch, Franco!

@geospatialem
Copy link
Member

It does look like this mitigates the multiple loaders, but there still seems to be a missing loader when the component is collapsed (not open) with 1.5.0-next.9: https://codepen.io/geospatialem/pen/poQPgNG.

Observe the first example does not have a loading indicator, but the open one does.

Would the icon slot need to account for both loading and when the component is not open?

@ashetland
Copy link
Contributor

Yeah that was the idea, that the icon slot would accommodate the loader in both the open and closed states. Any slotted icon would also get overridden when it's loading.

@Elijbet
Copy link
Contributor Author

Elijbet commented Jul 3, 2023

Cool, I'll follow up on this issue with the last comments. @ashetland @geospatialem

@geospatialem geospatialem mentioned this pull request Jul 9, 2023
Closed
@Elijbet Elijbet mentioned this pull request Jul 10, 2023
Merged
Elijbet added a commit that referenced this pull request Jul 12, 2023
@Elijbet
fix(block): loader now appears for all loading cases (#7303)
5af3600 
**Related Issue:** #6485

## Summary
Continues on #7239 to cover additional cases where loader needs to
appear when there is no `status icon` or if there is a `slottedIcon` so
that the loading icon shows both when a collapsible block is `open` and
`closed`. Any slotted icons get overridden when loading.
@ashetland ashetland mentioned this pull request Jul 12, 2023
Closed
3 tasks
benelan pushed a commit that referenced this pull request Aug 3, 2023
@calcite-admin @github-actions
chore: release latest (#7261)
45b5c01 
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-components: 1.5.0</summary>

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-08-03)


### Features

* **action-group:** Adds overlayPositioning property.
([#7366](#7366))
([ca9f35a](ca9f35a))
* Allow sharing focus trap stacks via configuration global
([#7334](#7334))
([934a19f](934a19f))
* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))
* **block, block-section:** Add setFocus method
([#7208](#7208))
([35d4bbb](35d4bbb))
* **block:** Improve block's content layout to allow scrolling
([#7367](#7367))
([ecbf17b](ecbf17b))
* **color-picker:** Replaces thumb focus outline to rounded
([#7378](#7378))
([d803980](d803980))
* **filter:** Add filter method
([#7127](#7127))
([5a4283f](5a4283f))
* **flow:** Adds setFocus method
([#7252](#7252))
([2472c58](2472c58))
* Improve focus behavior in components
([#7277](#7277))
([ad9fbca](ad9fbca))
* **input-time-zone:** Add input-time-zone component
([#6947](#6947))
([87bd496](87bd496))
* **list:** Add slots for filter actions
([#7183](#7183))
([da07ab1](da07ab1))
* **list:** Add support for dragging items.
([#7109](#7109))
([7324f70](7324f70))
* **menu-item:** Update spacing and icon layout
([#7381](#7381))
([5659671](5659671))
* **navigation-logo:** Increase font-size of heading with no description
([#7081](#7081))
([355e101](355e101))
* **switch:** Updates focus outline to be rounded
([#7390](#7390))
([2616b82](2616b82))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7299](#7299))
([c5678eb](c5678eb))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7412](#7412))
([c1af3c7](c1af3c7))


### Bug Fixes

* **accordion, accordion-item:** `icon-position`, `icon-type`,
`selection-mode` and `scale` can now be set as props or attributes
([#7191](#7191))
([2b09aba](2b09aba))
* **action-bar:** No longer delegates focus when clicked on
non-focusable region
([#7310](#7310))
([1a9c15c](1a9c15c))
* **action:** Correctly focus the button after rendering updates.
([#7255](#7255))
([40fe2ce](40fe2ce))
* **block:** Loader now appears for all loading cases
([#7303](#7303))
([5af3600](5af3600))
* **block:** Removes extra loading indicator
([#7239](#7239))
([a334a75](a334a75))
* **card:** Ensure teardown logic is called when disconnected
([#7289](#7289))
([d07e322](d07e322))
* **chip:** Disconnect mutation observer when component is disconnected
from the DOM
([#7418](#7418))
([412e5fb](412e5fb))
* **color-picker:** Draw slider thumbs within bounds
([#7398](#7398))
([2f37854](2f37854))
* **color-picker:** Fix opacity slider keyboard nudging
([#7400](#7400))
([2b4f7c3](2b4f7c3))
* **color-picker:** Maintains correct numbering system when entering
invalid RGB value
([#7327](#7327))
([8d2a3a5](8d2a3a5))
* **combobox:** Add space after grouped items
([#7302](#7302))
([b1580c7](b1580c7))
* **dropdown-item:** Provides accessible label when href is not parsed
([#7316](#7316))
([966b83d](966b83d))
* **flow:** Call setFocus() on back button click
([#7285](#7285))
([9102aa4](9102aa4))
* **input-date-picker:** Provides placeholder text context for AT users
([#7320](#7320))
([31e0ba2](31e0ba2))
* **input-date-picker:** Reset active date picker date after closing
([#7219](#7219))
([91b2a1b](91b2a1b))
* **input, input-number:** No longer removes trailing decimal separator
([#7159](#7159))
([01535cf](01535cf))
* **link:** Adds outline-offset to avoid overlapping with text.
([#7342](#7342))
([c30db4e](c30db4e))
* **list:** Changing filterText property will now update filtered items
([#7133](#7133))
([a9c0bce](a9c0bce))
* **list:** Fix keyboard navigation after a listItem's disabled or
closed property changes
([#7275](#7275))
([91d28eb](91d28eb))
* **list:** Fix keyboard navigation when filterEnabled is true
([#7385](#7385))
([41a2e42](41a2e42))
* **menu-item:** Prevent duplicate border in nested vertical menu-items
([#7387](#7387))
([186a738](186a738))
* **panel:** Remove double border styling when content isn't provided
([#7368](#7368))
([91a0610](91a0610))
* Remove style modifying all host components with hidden attribute
([#7346](#7346))
([3103e2f](3103e2f))
* **scrim:** Update loader scale on resize of component.
([#7419](#7419))
([24e7f70](24e7f70))
* **slider:** Prevent excessive tick rendering
([#7421](#7421))
([c799409](c799409))
* **switch:** Fix for focus outline style in certain cases
([#7414](#7414))
([217324f](217324f))
* **tab-title:** Add full focus outline to closable tab button in high
contrast mode
([#7272](#7272))
([d812d17](d812d17))
* **tooltip:** Avoid extra before open/close event emitting
([#7422](#7422))
([dbb6818](dbb6818))
* **tooltip:** Deprecate the label property due to the description
coming from the component's content
([#7247](#7247))
([7934d75](7934d75))
* **tooltip:** Emits `close` and `beforeClose` events when container is
set to `display:none`
([#7258](#7258))
([60a4683](60a4683))
* **tooltip:** Ensure --calcite-app-z-index-tooltip is applied
([#7345](#7345))
([a9a7072](a9a7072))
</details>

<details><summary>@esri/calcite-components-react: 1.5.0</summary>

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-08-03)


### Features

* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^1.5.0-next.38 to ^1.5.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driskull driskull driskull approved these changes

@jcfranco jcfranco jcfranco approved these changes

Assignees

@Elijbet Elijbet

Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug.
Projects
None yet
Milestone
2023 July Priorities
Development

Successfully merging this pull request may close these issues.
6 participants
@Elijbet @geospatialem @jcfranco @ashetland @driskull @benelan