-
Notifications
You must be signed in to change notification settings - Fork 310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DRAFT] Search components integration #2418
Conversation
…rosoft-graph-toolkit into next/fluentui # Conflicts: # index.html
Thank you for creating a Pull Request @FranckyC. This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:
|
@FranckyC there are a lot of edits on "untouched" files. Can you look into it? We'll be looking into it with the team and evaluate better the next steps! THANKS FOR THE CONTRIBUTION!!! |
*/ | ||
public static decode(encodedStr: string) { | ||
const domParser = new DOMParser(); | ||
const htmlContent: Document = domParser.parseFromString(`<!doctype html><body>${encodedStr}</body>`, 'text/html'); |
Check warning
Code scanning / CodeQL
Unsafe HTML constructed from library input
return /((([A-Za-z]{3,9}:(?:\/\/)?)(?:[\-;:&=\+\$,\w]+@)?[A-Za-z0-9\.\-]+|(?:www\.|[\-;:&=\+\$,\w]+@)[A-Za-z0-9\.\-]+)((?:\/[\+~%\/\.\w\-_]*)?\??(?:[\-\+=&;%@\.\w_]*)#?(?:[\.\!\/\\\w]*))?)/.test( | ||
url | ||
); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data
@sebastienlevert this is probably because I merged my forked next/fluentui from my branch that was based on main in the first place so the diff comes from here. Still, the goal is not to merge this one, just for you to test. Once the strategy defined, I'll start from scratch from latest |
I'm sorry, I messed up here, I was cleaning up the old feature branch for v3 and it closed this PR when the target branch was deleted. Can you please create a new PR targeting |
@gavinbarron done: #2582 |
PR Type
Feature
Description of the changes
Hi @gavinbarron, @sebastienlevert , as discussed, here is a first functional version of search components integration:
There is still lot of work to do but, at a glance, here are the following changes I've made to make it work:
Tailwind
Added Tailwind CSS integration with postcss and JIT mode. This allow components to use Tailwind styles on demand through dynamically generated
tailwind-styles.css
file. Just useyarn start
as usual and see the content of the file to updated automatically when you add new classes on your components.This relies on your existing SASS/TS modules transform mechanism. See
tailwind
task ingulpfile.js
. Typically, for a component it means:Dynamic imports
For search filters and performances purpose, I need to load correct locale according to the current language using daysJs library. However, because the current solution is not configured to have multiple bundles, I need to add the
inlineDynamicImports
flag on the Rollup config to keep my code as is (only on es6 config for now).As discussed, this is something that needs to be considered in the future to optimize distribution and performances.
Tests
Added test example for search verticals component with
@open-wc/testing
package, web test runner with esplugin/playwright.Added also
useDefineForClassFields
on a customtsconfig.test.json
to make it work with lit (I'm not using Babel).To run tests and watch mode just use
yarn wtr:watch
and the VSCode debug configuration "Tests debug (Edge)":Localization
In our company scenario, localization works for both components internal labels but also settings via attributes (ex: vertical name in multiple languages). For setting values, we define the
ILocalizedString
interface relying on a mandatory "language" property in localization strings:Normally, it is linked to a language provider component globally available on the page and setting this property using the LocalizationHelper but I didn't include it in this demo but you get the idea. Maybe something to think about in the future.
Next steps
Before going forward, we need to decide how to:
PR checklist
yarn build
) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense
)Other information