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

The role code is never executed in base MarkdownButtonElement #70

Open
wxiaoguang opened this issue Apr 11, 2023 · 2 comments
Open

The role code is never executed in base MarkdownButtonElement #70

wxiaoguang opened this issue Apr 11, 2023 · 2 comments

Comments

@wxiaoguang
Copy link

The role code is never executed in base MarkdownButtonElement

    connectedCallback() {
        if (!this.hasAttribute('role')) {
            this.setAttribute('role', 'button');
        }
    }

Because other classes like MarkdownHeaderButtonElement extends MarkdownButtonElement never call parent class's connectedCallback
@wxiaoguang wxiaoguang mentioned this issue Apr 11, 2023
Merged
lunny pushed a commit to go-gitea/gitea that referenced this issue Apr 11, 2023
@wxiaoguang
Fine tune markdown editor toolbar (#24046)
704f3aa 
1. Remove unnecessary `btn-link` `muted` classes
* Link is link, button is button, I can't see a real requirement to make
a button like a link.
* If anyone insists, please help to show me real example from modern
frameworks / websites, how and why they do so.
    * No need to duplicate a lot of class names on similar elements
* Declare styles clearly, for example, `markdown-toolbar` itself should
have `display: flex`, but not use `gt-df` to overwrite the `display:
block`.
2. Remove unnecessary `role` attribute
    * github/markdown-toolbar-element#70
* The `markdown-toolbar-element` does want to add `role=button`, but
there is a bug.
* So we do the similar thing as upstream does (add the role by JS),
until they fix their bugs.
3. Indent `markdown-switch-easymde` (before it doesn't have a proper
indent)

Screenshot:

![image](https://user-images.githubusercontent.com/2114189/231090912-f6ba01cb-d0eb-40ad-bf8c-ffc597d9a778.png)
@keithamus
Copy link
Member

Great catch! PRs welcome for sub components to call super.

@wxiaoguang
Copy link
Author

wxiaoguang commented May 3, 2023 •
edited
Loading

I haven't gotten time for it, feel free to fix it by your plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@keithamus @wxiaoguang