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(NcAppNavigation): change h2 to span #5020

Merged
merged 1 commit into from Jan 4, 2024
Merged

fix(NcAppNavigation): change h2 to span #5020

merged 1 commit into from Jan 4, 2024

Conversation

emoral435
Copy link
Contributor

@emoral435 emoral435 commented Jan 2, 2024 •
edited

☑️ Resolves

🖼️ Screenshots

🏚️ Before 🏡 After
293754098-54ff5621-bb32-40f8-bdbe-a05b764b005f image

🚧 Summary

Change the h2 element in NcAppNavigationCaption to span, as the former h2 element broke the hierarchy of headings within the user page.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable (NA)
  • 📘 Component documentation has been extended, updated or is not applicable (NA)

@emoral435 emoral435 added bug Something isn't working 3. to review Waiting for reviews component Component discussion and/or suggestion accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jan 2, 2024
@emoral435 emoral435 self-assigned this Jan 2, 2024
ShGKme
ShGKme requested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

It also changes styles (margin)

@ShGKme ShGKme changed the title fix(NcAppNav): changed h2 to span fix(NcAppNavigation): change h2 to span Jan 2, 2024
@emoral435
fix(NcAppNav): changed h2 to span
ccdd40f 
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
@emoral435 emoral435 force-pushed the fix/settings/42554/replace-h2-to-span branch from 198a676 to ccdd40f Compare January 2, 2024 22:15
@emoral435
Copy link
Contributor Author

I see what you mean - nice catch! The span needs to have 12px of margins on the bottom for it to be the exact same

h2 span
firefox_k9f0jORbog firefox_SvArjhQGvu

newest commit has this change correct, thank you!

@emoral435 emoral435 requested a review from ShGKme January 2, 2024 22:16
@JuliaKirschenheuter JuliaKirschenheuter merged commit 920f6b7 into master Jan 4, 2024
15 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/settings/42554/replace-h2-to-span branch January 4, 2024 16:57
@Pytal Pytal mentioned this pull request Jan 23, 2024
Merged
@Pytal Pytal mentioned this pull request Feb 21, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@Pytal Pytal Pytal approved these changes

@ShGKme ShGKme ShGKme approved these changes

Assignees

@emoral435 emoral435

Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working component Component discussion and/or suggestion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BITV]: Replace <h2> element from 'Users' view to span
4 participants
@emoral435 @JuliaKirschenheuter @Pytal @ShGKme