fix(NcListItem): Remove actions from within a which caused invalid HTML output
#5198
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
susnux
added
bug
susnux
requested review from
Antreesy,
JuliaKirschenheuter,
Pytal,
ShGKme and
raimund-schluessler
susnux
force-pushed
the
fix/nc-list-item
branch
from
8c70a6a to
79ecd9a
Compare
susnux
changed the title
a which caused invalid HTML output
Pytal
reviewed
Jan 31, 2024
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5198 +/- ##
==========================================
- Coverage 39.34% 39.30% -0.05%
==========================================
Files 139 139
Lines 3688 3692 +4
Branches 810 811 +1
==========================================
Hits 1451 1451
- Misses 2021 2025 +4
Partials 216 216 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ferdinand Thiessen <[email protected]>
Contributor
susnux
force-pushed
the
fix/nc-list-item
branch
3 times, most recently
from
35eafd2 to
6e45c2e
Compare
Contributor
Author
|
Ready to review I would say 🚀 vokoscreenNG-2024-02-02_15-02-36.mp4 |
susnux
force-pushed
the
fix/nc-list-item
branch
from
6e45c2e to
2a27f08
Compare
JuliaKirschenheuter
approved these changes
Feb 2, 2024
Contributor
|
Compact is a little bit broken, the height is changed on hover.
|
ShGKme
reviewed
Feb 2, 2024
Contributor
ShGKme
left a comment
ShGKme
left a comment
There was a problem hiding this comment.
Works fine now with both Tab navigation and the screen reader quick navigation. The only issue I've found is the height on hover in compact mode
Contributor
Author
@ShGKme I fixed this now: vokoscreenNG-2024-02-02_16-03-38.mp4 |
ShGKme
approved these changes
Feb 2, 2024
Contributor
ShGKme
left a comment
ShGKme
left a comment
There was a problem hiding this comment.
Everything seems to work fine now, thanks!
…e to use of button within anchor Co-authored-by: Ferdinand Thiessen <[email protected]> Co-authored-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: Ferdinand Thiessen <[email protected]>
susnux
force-pushed
the
fix/nc-list-item
branch
from
1568a13 to
0886575
Compare
2 tasks
ShGKme
pushed a commit
that referenced
this pull request
Feb 5, 2024
fix(NcListItem): Remove actions from within `a` which caused invalid HTML output
Merged
Antreesy
reviewed
Feb 13, 2024
3 tasks
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
☑️ Resolves
It is not valid to put
buttoninsideathus we need to refactor the NcListItem:awithdiva, this is required so users can right click + copy link or middle click -> native browser behavior.Hint: Hide white space changes when review
🖼️ Screenshots
Could not spot visual changes except from focus visible.
a.mp4
🏁 Checklist