budibase icon indicating copy to clipboard operation
budibase copied to clipboard

Make Builder App Section Navigation Tabs Anchors

Open Ghrehh opened this issue 9 months ago • 3 comments

A lot of our links throughout the app aren't actual anchors, but are buttons that behave like links.

This isn't great for a few reasons, but one particular frustrating one for me was being unable to open the data section in a new tab from the design section while working on an app.

This change makes the builder app editing navigation actual links, while still maintaining the onClick functionality of the tab component.

If there's no issues with this we can easy do it for any other consumers of the tab component, and the same sort of change can be replicated across other link buttons in the app.

Ghrehh avatar May 07 '24 08:05 Ghrehh

Another way to handle this (which is applicable to all sorts of components without much in the way of code changes) would just be to detect the user holding down ctrl/cmd and then open a new tab.

The only changes required for any component like this is inside the on:click handler:

    if (e.ctrlKey || e.metaKey) {
      window.open($url(path), "_blank")
    } else {
      $goto(path)
    }

We also just need to ensure we have access to the native JS event parameter, and the best way to do that (if you already have a custom handler for on:click like we do in the Tab component) is add another on:click just to proxy it up.

So in the Tab component we can remove the custom dispatch("click") and instead just do

    on:click={onClick}
    on:click

That ensures that the on:click callback in the parent has the proper event param.

The main reason I prefer this approach is that it would work for absolutely any component with a click handler without having to add new markup for HTML link components into each of them.

aptkingston avatar May 13 '24 12:05 aptkingston

@aptkingston I understand where you're coming from, but the implementation you suggested doesn't do the following (I think):

  • On hover link previews.
  • Right-click link menu (or force touch on mobile etc)
  • Semantic html stuff for screen readers/search engine indexing etc
  • Probably some other stuff I'm not aware of.

I'm sure we could implement most if not all of that in JS, but at some point I think it makes sense to just use an anchor if it's anchor behaviour that we want/is expected 🤷

Though I also get that this is a change to a very important component for some pretty minor convenience/tidiness gains, so I'm happy to change it to your suggested implementation if you think the reduction to the risk of breaking something is worth the compromise.

Ghrehh avatar May 15 '24 08:05 Ghrehh

You're not wrong that it doesn't address those accessibility issues, but I was just going off your description where the primary motivation was being able to open links in new tabs. The builder doesn't support mobile either, but yea I'm happy to just leave this as it is then to address those extra points you mentioned.

aptkingston avatar May 15 '24 08:05 aptkingston