Skip to content

Add icon-gallery to devtest#37067

Draft
bircni wants to merge 7 commits intogo-gitea:mainfrom
bircni:feature/make-sqlite-watch
Draft

Add icon-gallery to devtest#37067
bircni wants to merge 7 commits intogo-gitea:mainfrom
bircni:feature/make-sqlite-watch

Conversation

@bircni
Copy link
Copy Markdown
Member

@bircni bircni commented Apr 1, 2026

When I was developing I sometimes added a devtest page locally for testing purposes, so here is my PR.
Add new pages to devtest:

  • icon-gallery

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 1, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/internal labels Apr 1, 2026
@wxiaoguang wxiaoguang marked this pull request as draft April 1, 2026 08:16
@wxiaoguang
Copy link
Copy Markdown
Contributor

It seems that most changes are not really needed.

@silverwind
Copy link
Copy Markdown
Member

Some screenshots would be helpful.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Some screenshots would be helpful.

According to my review, I think most "devpage" changes should be reverted or refactored.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 1, 2026

Yes, no duplicating of implementation details because that would incur extra effort to keep those implementations in sync. It should work on component level (similar to how Storybook can be used for component-level previews/demos/tests).

I guess our devtest pages could eventually be replaced with Storybook if desired, it's the same principle.

@wxiaoguang
Copy link
Copy Markdown
Contributor

I guess our devtest pages could eventually be replaced with Storybook if desired, it's the same principle.

There are plenty of "golang template components" or "heavy JS logic component" tested in devtest pages.

  • "repo/commit_sign_badge"
  • class="link-action" / class="form-fetch-action"
  • modal ("base/modal_actions_confirm")
  • "shared/combomarkdowneditor"
  • mail templates
  • markup render
  • "repo/actions/view_component"
  • toast

Will Storybook work for them?

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 1, 2026

Will Storybook work for them?

Storybook is primarily for testing UI components (Vue, React) in isolation without a backend, so not a a good fit for backend-based components, but a good fit for Vue components (with relevant mocking).

@wxiaoguang
Copy link
Copy Markdown
Contributor

Will Storybook work for them?

Storybook is primarily for testing UI components (Vue, React) in isolation without a backend, e.g. if backend is needed I would probably mock it to retain the isolation.

None of these is "UI components (Vue, React)"

@bircni bircni force-pushed the feature/make-sqlite-watch branch from 71f3e5a to 74c1dff Compare April 1, 2026 09:53
@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 1, 2026

Updated:
Only icon gallery and avatars are left

@bircni bircni marked this pull request as ready for review April 1, 2026 09:53
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 1, 2026

avatars are left

Just curious, but we view the avatars everyday on the UI, they are almost on every page, why it needs a separate devtest page?

@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 1, 2026

avatars are left

Just curious, but we view the avatars everyday on the UI, they are almost on every page, why it needs a separate devtest page?

agree... removing too

@bircni bircni changed the title Add more devtest pages Add icon-gallery to devtest & watch-sqlite make target Apr 1, 2026
@bircni bircni changed the title Add icon-gallery to devtest & watch-sqlite make target Add icon-gallery to devtest Apr 1, 2026
@bircni bircni requested review from silverwind and wxiaoguang April 1, 2026 18:09
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 1, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

@silverwind do you agree to use inline scripts and inline styles?

@wxiaoguang wxiaoguang marked this pull request as draft April 1, 2026 22:16
@wxiaoguang
Copy link
Copy Markdown
Contributor

By the way, I think we should drop the imports of the "standalone" JS scripts, merge them into "index.ts" and import them when needed, because the vite can chunk them correctly.

The "standalone" modules will cause double-import problem (see my comment in the Webpack to Vite PR)

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 2, 2026

  1. No, move to typescript/tailwind
  2. Could be done yeah, will be less bundle size and chunks at the cost of running a bit of unnecessary JS on those standalone pages.

@silverwind silverwind self-requested a review April 2, 2026 09:31
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 2, 2026
@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 5, 2026

So what do I need to do? what of that is part of this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants