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

[web] Agama/Section fixes and improvements #892

Merged
merged 10 commits into from
Nov 30, 2023

Conversation

dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Nov 28, 2023

Problem

Lately, we use more and more the Agama/Section component, which make us realize about problems it has or improvements we can introduce. This PR is about both things.

Details

  • After [web] Simplify code of Section component #839, there are a lot of Icon errors logged in the development console when visiting a page with sections without icons.

    Fixed by 59ff592

  • There are sections that do not have a representative icon, but they can enter in loading mode. When so, the loading icon is used in the section header, moving the section header and content to the right as much as needed and putting it back when leaving the loading mode.

    Solution has been to revert change done at [web] Do not reserve space for section icons #549 and reserve the icon space, 1672dbc

    BeforeAfter

    Screenshot from 2023-11-29 09-57-05

    Screenshot from 2023-11-29 10-04-25

    Screencast.from.2023-11-29.09-58-18.webm

    Screencast.from.2023-11-29.09-59-54.webm

  • Because of the above, better to add padding to the end of the section for a visual balance of space at both sides ac74693

    Before After
    Screenshot from 2023-11-29 10-04-25 Screenshot from 2023-11-29 10-17-12
  • But there is too much wasted space, so the padding added by the <main> element was reduced, c62976c

    Before After
    Screenshot from 2023-11-29 12-38-29 Screenshot from 2023-11-29 12-38-00
  • Still too much space when sections do not have icons, so icons size was decreased too, 09961b8

    Before After
    Screenshot from 2023-11-29 12-44-41 Screenshot from 2023-11-29 12-45-39
    Screenshot from 2023-11-29 12-45-09 Screenshot from 2023-11-29 12-45-29
  • Then the header had to be better aligned, 6cb6512

    Before After
    Screenshot from 2023-11-29 12-48-50 Screenshot from 2023-11-29 12-49-10
  • And finally ensuring that sidebar content keep always visible, 0927e87

    Before After
    Screenshot from 2023-11-29 12-56-38 Screenshot from 2023-11-29 12-56-51

Because if called with undefined name Agama/Icon logs an error in the
console.
A kind of rollback of #549 to
avoid UI changes when a section without icon enters in a loading mode
and renders the "section loading icon".
The purpuose is to keep a visual balance with the space reserved for the
section / loading icon.
To make better use of space now that page sections are adding their own
inline padding.
To make the section icons smaller by default, reducing the left gap
generated when using sections without icons.
Changes in the --wrapper-padding make it being cut in the left side.
@coveralls
Copy link

coveralls commented Nov 28, 2023

Coverage Status

coverage: 75.554% (+0.009%) from 75.545%
when pulling e1dd886 on section-improvements-20231127
into 5e7a449 on master.

As a better way to avoid its content to be hidden or cut.
@dgdavid dgdavid marked this pull request as ready for review November 29, 2023 13:04
@dgdavid dgdavid changed the title Section improvements 20231127 [web] Agama/Section fixes and improvements Nov 29, 2023
Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit c47fc31 into master Nov 30, 2023
1 check passed
@dgdavid dgdavid deleted the section-improvements-20231127 branch November 30, 2023 11:57
@imobachgs imobachgs mentioned this pull request Dec 2, 2023
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