feat(browser): browser history and tab favicons#413
Merged
nicoburns merged 3 commits intoDioxusLabs:mainfrom May 2, 2026
Merged
feat(browser): browser history and tab favicons#413nicoburns merged 3 commits intoDioxusLabs:mainfrom
nicoburns merged 3 commits intoDioxusLabs:mainfrom
Conversation
Adds a browsing history page (about:history) with per-entry favicons and elapsed-time labels, favicon display in the tab strip, and ICO decoder support. Favicon probing and sniffing live in a new favicon.rs module; history data and dedup logic are in browser_history.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nicoburns
requested changes
May 2, 2026
Member
nicoburns
left a comment
nicoburns
left a comment
There was a problem hiding this comment.
This is mostly looking good. However, I have left some review feedback.
Member
|
There seems to be an issue with the UI layout when there are enough history entries to cause the page to scroll: Screen.Recording.2026-05-02.at.22.07.00.mov |
…ippy cleanup - Replace Signal<VecDeque<HistoryEntry>> with Store<BrowsingHistory>; record_visit/clear become Store extension methods. The free function is kept private so it can be unit-tested without a Dioxus runtime. - Favicon probe decodes via image+usvg instead of magic-byte sniffing, so truncated payloads are correctly rejected. Drop the `ico` cargo feature in favor of always-on image/ico. - Add BaseDocument::favicon_url() so the browser app reads the icon link through the document API. - .tab-content: min-height: 0 to let the flex child shrink. - apps/browser is clean under -W unwrap_used/expect_used/panic/ indexing_slicing/arithmetic_side_effects/redundant_clone: saturating arithmetic for indices, wrapping_add for the reload generation counter, get/get_mut for bounded indexing, let-else for Event downcasts, scoped #[allow] with reason comments where an invariant makes the operation safe. capture_anyrender_scene logs and returns on error instead of unwrapping. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
nicoburns
requested changes
May 2, 2026
Member
nicoburns
left a comment
nicoburns
left a comment
There was a problem hiding this comment.
The favicon, history store changes and layout fix look good.
However, this commit contains a lot of drive-by lint-related changes again. Can we remove those? (and somehow instruct Claude not to keep introducing them?). It's important for maintaintability by humans that the code stays readable, and making 1 liners into 3 or 4 liners does not help with that!
|
|
||
| #[store(pub)] | ||
| impl<Lens> Store<BrowsingHistory, Lens> { | ||
| fn record_visit(&self, entry: HistoryEntry) |
Member
There was a problem hiding this comment.
I believe if you use &mut self then you don't need the where Lens: Writable
| let mut scene = anyrender::Scene::new(); | ||
| render_scene(doc, &mut scene, RenderSize::Viewport); | ||
|
|
||
| let config = SerializeConfig::new() |
Member
There was a problem hiding this comment.
The changes in this file are seem unrelated to the rest of the PR and are not needed. Please undo.
Comment on lines
76
to
79
| let mut reload_generation = self.reload_generation; | ||
| *reload_generation.write() += 1; | ||
| let next = (*reload_generation.read()).wrapping_add(1); | ||
| *reload_generation.write() = next; | ||
| } |
Member
There was a problem hiding this comment.
No need for wrapping_add. Please undo.
Comment on lines
29
to
52
| if let Some(prev) = self.last { | ||
| let dt = now.duration_since(prev); | ||
| self.ring[self.idx] = dt; | ||
| self.idx = (self.idx + 1) % RING_LEN; | ||
| if let Some(slot) = self.ring.get_mut(self.idx) { | ||
| *slot = dt; | ||
| } | ||
| self.idx = self.idx.wrapping_add(1) % RING_LEN; | ||
| if self.count < RING_LEN { | ||
| self.count += 1; | ||
| self.count = self.count.wrapping_add(1); | ||
| } | ||
| } | ||
| self.last = Some(now); | ||
| } | ||
|
|
||
| fn snapshot(&self) -> (f32, f32) { | ||
| if self.count == 0 { | ||
| return (0.0, 0.0); | ||
| } | ||
| let total: Duration = self.ring[..self.count].iter().copied().sum(); | ||
| let avg = total / self.count as u32; | ||
| let slice = self.ring.get(..self.count).unwrap_or(&[]); | ||
| let total: Duration = slice.iter().copied().sum(); | ||
| // count > 0 by the early return above, so division is safe. | ||
| let avg = total | ||
| .checked_div(self.count as u32) | ||
| .unwrap_or(Duration::ZERO); | ||
| let ms = avg.as_secs_f32() * 1000.0; |
Comment on lines
+117
to
+118
| // We just pushed; the last element is the tab we want. | ||
| #[allow(clippy::expect_used)] |
Member
There was a problem hiding this comment.
Why have all of these popped up, even though that setting isn't in the clippy config anymore?
Signed-off-by: Nico Burns <nico@nicoburns.com>
Member
Realised I could quite easily do this myself using git. |
nicoburns
approved these changes
May 2, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds a browsing history page (
about:history) with per-entry favicons and elapsed-time labels, and favicon display in the tab strip. Favicon probing/sniffing lives in a newfavicon.rsmodule; history data and dedup logic are inbrowser_history.rs.