2nd round code review fixes

Author: schu96

package-lock.json

@@ -42,6 +42,7 @@
     "@internetarchive/icon-magnify-minus": "^1.4.0",
     "@internetarchive/icon-magnify-plus": "^1.4.0",
     "@internetarchive/icon-search": "^1.4.0",
+    "@internetarchive/icon-share": "1.4.0",
     "@internetarchive/icon-toc": "^1.4.0",
     "@internetarchive/icon-visual-adjustment": "^1.4.0",
     "@internetarchive/modal-manager": "^0.2.12",

package.json

@@ -152,17 +152,61 @@
     display: none;
 }
 
+.br-selection-menu__root {
+  display: none;
+  border-radius: 20px;
+  background-color: #333;
+  color-scheme: dark;
+  padding: 2px;
+  overflow: hidden;
+  transition: border-radius 0.2s;
+}
+.br-selection-menu__root:hover {
+  border-radius: 8px;
+}
+
+.br-selection-menu__root:hover .textFragmentButton {
+  border-radius: 6px;
+}
+
 .textFragmentButton {
-    background: inherit;
-    background-position: center;
-    background-repeat: no-repeat;
-    background-color: darksalmon;
-    width: 60px;
-    height: 30px;
-    top: inherit;
-    left: inherit;
+  --iconWidth: 15px;
+  --iconHeight: 15px;
+  --iconFillColor: currentColor;
+  background: transparent;
+  display: flex;
+  align-items: center;
+  border-radius: 20px;
+  font-family: inherit;
+  border: 0;
+  transition: background-color 0.2s, border-radius 0.2s;
+  cursor: pointer;
+  text-wrap: nowrap;
+  padding: 4px;
 }
 
-.textFragmentButtonBar {
-    display: none;
+.textFragmentButton:hover {
+  background-color: rgba(255, 255, 255, 0.1);
+}
+
+.textFragmentButton ia-icon-share {
+  display: block;
+  flex-shrink: 0;
+  width: 17px;
+  height: 17px;
+}
+
+.menuOptionTitle {
+  width: 0px;
+  opacity: 0;
+  interpolate-size: allow-keywords;
+  transition: width 0.2s;
+  font-size: 12px;
+}
+
+
+.br-selection-menu__root:hover .menuOptionTitle {
+  width: auto;
+  margin-left: 4px;
+  opacity: 1;
 }

src/css/_TextSelection.scss

@@ -51,17 +51,17 @@ export class ExperimentsPlugin extends BookReaderPlugin {
     localStorageKey: 'BrExperiments',
 
     /** The experiments that should be shown in the experiments panel */
-    enabledExperiments: ['translate', 'highlightUrl'],
+    enabledExperiments: ['translate', 'copyToSelectionUrl'],
   }
 
   /** @type {ExperimentModel[]} */
   allExperiments = [
     new class extends ExperimentModel {
-      name = 'highlightUrl';
-      title = 'URL Link to Highlight';
-      description = 'Share highlighted content via URL';
+      name = 'copyToSelectionUrl';
+      title = 'Copy to Selection URL';
+      description = 'Share text selection via URL';
       learnMore = 'none';
-      icon = 'images/translate.svg';
+      icon = null;
       enabled = false;
       async enable ({ manual = false }) {
         this.br.plugins.textSelection.enableExperiment();

src/plugins/plugin.experiments.js

@@ -64,9 +64,8 @@ export class TextSelectionPlugin extends BookReaderPlugin {
   }
 
   enableExperiment() {
-    if (!this.textSelectionManager) return;
-    this.textSelectionManager.highlightEnabled = true;
-    this.textSelectionManager.addHighlightBar();
+    this.textSelectionManager.selectionMenuEnabled = true;
+    this.textSelectionManager.renderSelectionMenu();
   }
 
   /**

src/plugins/plugin.text_selection.js

@@ -171,10 +171,7 @@ BookReader.prototype.urlUpdateFragment = function() {
       const newFragmentWithSlash = newFragment === '' ? '' : `/${newFragment}`;
       const textFragment = this.urlPlugin.retrieveTextFragment(newQueryString);
       const newUrlPath = `${baseWithoutSlash}${newFragmentWithSlash}${newQueryString}`;
-      let extractedPage = newFragmentWithSlash.match(/(?<=\/)n?\d+(?=\/)/);
-      if (extractedPage) {
-        extractedPage = extractedPage[0];
-      }
+      const extractedPage = this.urlPlugin.urlStringToUrlState(newFragmentWithSlash)?.page;
       if (!this.textFragmentPage && textFragment) {
         this.textFragmentPage =  extractedPage ? extractedPage : null;
         this.textFragment = `:~:text=${textFragment}`;
@@ -261,14 +258,13 @@ export class BookreaderUrlPlugin extends BookReader {
         this.on('textLayerVisible', async (_, {pageContainerEl}) => {
           const visiblePageNum = pageContainerEl.getAttribute('data-page-num');
           await new Promise(resolve => setTimeout(resolve, 100));
+          // No textFragment found or the textFragment stored doesn't match current visible page loaded
+          if (!this.textFragment || this.textFragmentPage !== visiblePageNum) return;
           if (this.options.urlMode === 'history') {
-            if (this.textFragment && this.textFragmentPage == visiblePageNum) {
-              window.location.replace(`#${this.textFragment}`);
-            }
+            window.location.replace(`#${this.textFragment}`);
           } else {
-            if (this.textFragment && this.textFragmentPage == visiblePageNum) {
-              window.location.replace(`#${this.oldLocationHash}`);
-            }
+            // for urlMode hash, textFragment is stored in oldLocationHash already
+            window.location.replace(`#${this.oldLocationHash}`);
           }
         });
       }