Skip to content

Reduce duplicate code - button like anchor#1063

Merged
Arnei merged 21 commits intoopencast:mainfrom
Arnei:buttonlikeanchor
Mar 20, 2025
Merged

Reduce duplicate code - button like anchor#1063
Arnei merged 21 commits intoopencast:mainfrom
Arnei:buttonlikeanchor

Conversation

@Arnei
Copy link
Copy Markdown
Member

@Arnei Arnei commented Jan 15, 2025

We have many buttons using the "button-like-anchor" class. This patch introduces a button component for these buttons, with built-in tooltip and access check to better support common code combinations for those buttons. In and of itself not a particularly impressive component, but the underlying goal here is to push the app more towards using common basic components.

How to test this

Can be tested as usual.

@Arnei Arnei added the type:code-enhancement Internal improvements to the codebase label Jan 15, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 15, 2025

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-1063

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-1063

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 15, 2025 •
edited
Loading

This pull request is deployed at test.admin-interface.opencast.org/1063/2025-03-20_10-33-11/ .
It might take a few minutes for it to become available.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 16, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@Arnei
Reduce duplicate code - button like anchor
844dd74 
We have many buttons using the "button-like-anchor"
class. This patch introduces a button component
for these buttons, with built-in tooltip and access
check to better support common code combinations
for those buttons.
In and of itself not a particularly impressive
component, but the underlying goal here is to
push the app more towards using common basic
components.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 21, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 31, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 31, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 31, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 5, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 11, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@Arnei
Fix missing import
c0b6d17 
Due to updating React we now need an explicit import.
ferishili
ferishili reviewed Mar 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ferishili ferishili left a comment

Choose a reason for hiding this comment

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

Thanks @Arnei
I have provided some inline suggestions, which could be replaced easily!
However, in some places I could still find the old button-like-anchor like:

Comment thread src/components/events/partials/ModalTabsAndPages/NewTobiraPage.tsx Outdated
Comment thread src/components/events/partials/PublishedCell.tsx Outdated
Comment thread src/components/events/partials/modals/EmbeddingCodeModal.tsx Outdated
Comment thread src/components/events/partials/modals/EmbeddingCodeModal.tsx Outdated
Comment thread src/components/events/partials/modals/EmbeddingCodeModal.tsx Outdated
Comment thread src/components/events/partials/modals/EmbeddingCodeModal.tsx Outdated
Comment thread src/components/events/partials/modals/EmbeddingCodeModal.tsx Outdated
Comment thread src/components/shared/Table.tsx Outdated
@ferishili ferishili self-requested a review March 13, 2025 07:50
ferishili
ferishili suggested changes Mar 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ferishili ferishili left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Forgot to submit my request changes! 😄 (all above)

Arnei and others added 9 commits March 13, 2025 13:24
@Arnei @ferishili
Fix indentation
be6616e 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
19da72e 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
6743623 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
df790a6 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
d4bd4de 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
f12bf50 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix duplicate classname
8648823 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei @ferishili
Fix indentation
63a088b 
Co-authored-by: FarbodZamani <53179227+ferishili@users.noreply.github.com>
@Arnei
More buttonlikeanchor
e4c5461 
 Two more instances were we can replace
raw button tags with buttonlikenachor.
@Arnei
Copy link
Copy Markdown
Member Author

Arnei commented Mar 13, 2025

Thanks for reviewing! Added your suggestions.

The only thing I did not change was the occurence of button-like-anchor in IconButton.tsx. IconButton is somewhat similar to ButtonLikeAnchor, but not quite the same. I would like to figure out what to do with those two button components in a different PR if you don't mind. (Should they be in some sort of hierarchy? Should one replace the other outright? Those kind of questions.)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 18, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

ferishili
ferishili approved these changes Mar 19, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ferishili ferishili left a comment

Choose a reason for hiding this comment

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

Thanks @Arnei
LGTM

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 20, 2025

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@Arnei Arnei merged commit 61d85fa into opencast:main Mar 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ferishili ferishili ferishili approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type:code-enhancement Internal improvements to the codebase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Arnei @ferishili