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

fix(settings): apps list html validation and loading icon #41035

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 21, 2023

Summary

  • Error: An img element must have an alt attribute, except under certain conditions.
    • Add empty alt. It is fine as the app image has no important information in addition to name and description
  • Error: Bad value 100% for attribute width on element img: Expected a digit but saw % instead.
    • 100% attribute value is not valid. Use CSS styles instead HTML attribute.
  • Error: Attribute content-class not allowed on element div at this point.
    • There is no content-class prop on NcContent. Just removed, loading icon is already set on NcAppContent
  • Error: Attribute navigation-class not allowed on element div at this point.
    • There is no navigation-class prop on NcContent. Removed and set as a normal class on NcAppNavigation
    • It also fixes displaying loading icon in apps categories.
Before After
image image

Checklist

@susnux
Copy link
Contributor

susnux commented Oct 23, 2023

conflicts

@ShGKme ShGKme force-pushed the fix/37092/apps-list--html-validation branch from ed7a946 to 1d45bed Compare October 23, 2023 16:08
- Replace invalid `width="100%"` attribute
- Add empty required `alt`

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- fix setting loading classes
- also removes invalid HTML attributes, there is no such props in `NcContent`

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/37092/apps-list--html-validation branch from 1d45bed to 7e94468 Compare October 24, 2023 09:10
@susnux
Copy link
Contributor

susnux commented Oct 24, 2023

cypress unrelated

@susnux susnux merged commit 6753aaf into master Oct 24, 2023
39 of 42 checks passed
@susnux susnux deleted the fix/37092/apps-list--html-validation branch October 24, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants